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.

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

Bugfix/write to hdf5 #39

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Bugfix/write to hdf5 #39

wants to merge 3 commits into from

Conversation

yaoyic
Copy link
Collaborator

@yaoyic yaoyic commented Oct 24, 2023

PR Checklist

  • Bug fix
  • [] Feature addition/change
  • [] Documentation addition/change
  • [] Test addition/change
  • Black formatting

Describe your changes here:

Aldo reported the issue that an Ensemble could not be saved to HDF5 again after features fraction_native_contacts and rmsd are computed and attached to the ensemble. This is a tentative fix. Aldo could you check whether this is working and maybe add some test cases?

@sayeg84
Copy link
Collaborator

sayeg84 commented Oct 31, 2023

This is working but it still fails for other features such as rg. I realized this because I tried to make a test using the RG and the code broke. I could also wait for a general fix for being able to serialize a model that contains any of the features obtainable through a Featurizer.

Should we move this PR only for two features or maybe wait for one that fixes for all?

@yaoyic
Copy link
Collaborator Author

yaoyic commented Nov 1, 2023

@sayeg84 do you have a general test that test for all features. I am afraid there is no general fix to this, since a feature's metadata cannot be a dictionary with every possible type without any consequence. With that I mean sure we can try to pickle stuff up, but 1. metadata of a h5 dataset has limited space requirement and 2. pickling may depend on the python version and corresponding package version compatibility. It would be great if I had made a clearer definition for the interface regarding which metadata is valid or so, but for now we have to deal with them case by case.
For the time being, would you be able to contribute and push a few tests that covers all features, and then I will try to fix the bugs accordingly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants