Skip to content

Bugfix/write to hdf5#39

Open
yaoyic wants to merge 3 commits intomainfrom
bugfix/write_to_hdf5
Open

Bugfix/write to hdf5#39
yaoyic wants to merge 3 commits intomainfrom
bugfix/write_to_hdf5

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