Skip to content
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

zml.aio.torch: more refactor #59

Merged
merged 14 commits into from
Oct 18, 2024
Merged

zml.aio.torch: more refactor #59

merged 14 commits into from
Oct 18, 2024

Conversation

gwenzek
Copy link
Contributor

@gwenzek gwenzek commented Oct 17, 2024 •
edited
Loading

  • use more complicated test cases with long integers
  • add tests for protocol 0. Pickle as evolve a lot since protocol 0 was there,
    and it's unlikely we receive such pickle, I thought it was justified since a big part of the code is related to protocol 0 anyway.
  • add tests to the "zipped pickle"
  • simplify structs hierarchy -> PickleData has been merged with Parser into torch.File.
  • rename torch/value.zig -> torch/py_object.zig
  • intermediary results: []pickle.Op, []py_object.AnyPy, should not be stored inside objects but passed around function calls.
  • we should use buffered reader to parse the pickle and the zip headers
  • python Dict should not be represented by a singleton seq of seq of seq of kv, just a single seq with keys and values following each other is enough (like in pickle protocol)
  • kwargs should not be mixed with args
  • added harder pytorch test with a raw tensor and a torch.nn.Conv2d module
  • simplified a bit lifecylcle of BufferStore by passing the files during init.

cryptodeal
cryptodeal approved these changes Oct 18, 2024
View reviewed changes
Copy link
Contributor

@cryptodeal cryptodeal left a comment

Choose a reason for hiding this comment

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

Looks good; I like the reorganization/cleanup done here a lot!

@gwenzek
Copy link
Contributor Author

gwenzek commented Oct 18, 2024

I've made some extra changes. there are two TODO for you james in nemo.zig

@gwenzek gwenzek merged commit edc0454 into master Oct 18, 2024
1 check passed
@gwenzek gwenzek deleted the gw/torch_refactor branch October 18, 2024 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cryptodeal cryptodeal cryptodeal approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gwenzek @cryptodeal