Skip to content

Cleanup/Minimalization of C++ Reflection patch #4209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Cleanup/Minimalization of C++ Reflection patch #4209

wants to merge 1 commit into from

Conversation

jmikedupont2
Copy link

@jmikedupont2 jmikedupont2 commented Nov 25, 2023 •
edited
Loading

Working now, is much cleaner

@jmikedupont2 jmikedupont2 changed the title still not working but cleaner Cleanup/Minimalization of C++ Reflection patch Nov 25, 2023
This was referenced Nov 25, 2023
Closed
Closed
Galunid
Galunid suggested changes Nov 26, 2023
View reviewed changes
Copy link
Contributor

@Galunid Galunid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding turning ggml to C++ - definitely not an option.
But appreciate the effort in this PR and don't mind having it live as a separate branch.

#4155 (comment)

@Galunid
Copy link
Contributor

Galunid commented Nov 26, 2023

I'm not sure if we should create a separate branch in this repo and merge this PR there, or if we should just leave this PR open?

@jmikedupont2
Copy link
Author

so I have added some more commits that dump out more data as example, I am working on a slow trace of all the tensors value in there now. download (8).png

20231126_160823.jpg

working
77f4b99 
Naming the unnamed ggml structures

Here we add names for the nested structures of ggml

adding print statements to main.

This inserts the print probes at key points

adding include for refl

working

now server has it
@jmikedupont2
Copy link
Author

#4228 I split out the viz stuff into its own branch and rebased this. it cleanly rebased off of master. I am very happy with it.

@jmikedupont2 jmikedupont2 mentioned this pull request Nov 27, 2023
Closed
@ggerganov ggerganov closed this Dec 12, 2023
@jmikedupont2
Copy link
Author

finally some feedback after spending weeks on these patches. not very nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@Galunid Galunid Galunid requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jmikedupont2 @Galunid @ggerganov