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

Fix CI crashing with test_spacegroup #339

Merged
merged 3 commits into from
Aug 26, 2024
Merged

Conversation

AlexandraVolokhova
Copy link
Collaborator

I've updated the names of the spacegroups in gflownet/envs/crystals/space_groups.yaml such that they correspond to the new conventions in pymatgen=2024.8.9. All tests pass, but I'm not 100% sure this change doesn't affect some other things in the crystal-gfn. @alexhernandezgarcia or @carriepl if you have a moment to double check that it's all good, that would be very nice 🙏

Copy link
Collaborator

@carriepl carriepl 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 to me although I am not aware of everything that could break as a result of these changes. AFAIK it's fine but I don't know for a fact.

Also, interestingly, another test is failing here related to the batch class. To me, it seems unrelated to the current PR so it shouldn't stop the merge. But we should look at what test it is, what the problem is (based on the face that we haven't seen this before, it might be cause by a rare stochastic bug), and try to fix in another PR.

@AlexandraVolokhova
Copy link
Collaborator Author

yes, seems like this crashing is very stochastic, I've tried to fix it with stronger atol, it was still failing, then I removed it and it passed the tests. Meanwhile locally everything worked fine all the time.

@AlexandraVolokhova AlexandraVolokhova merged commit 98cc4f9 into main Aug 26, 2024
1 check passed
@AlexandraVolokhova AlexandraVolokhova mentioned this pull request Sep 25, 2024
5 tasks
swarnim-j added a commit to swarnim-j/gflownet that referenced this pull request Nov 10, 2024
Update .gitignore and add logs directory
Merge pull request alexhernandezgarcia#339 from alexhernandezgarcia/fix-pymagen-test
Remove atol from test
Fix failing batch test
Refactor code to save figures in evaluator
Update GFlowNetEnv class to call reset() before setting initial state and action space
Update base.yaml and pruning.yaml with default settings for pruning experiment
Add train_lenet.py script for training and evaluating LeNet model on CIFAR-10 dataset
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