-
Notifications
You must be signed in to change notification settings - Fork 0
Random sample #13
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
base: main
Are you sure you want to change the base?
Random sample #13
Conversation
stephprince
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added my comments! I was mainly reviewing for any changes that would break the core model training so I did not look in detail at the refinement_model module updates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this file should be included
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this and debug_train_structure.sh and test_imports.py are leftovers from an AI debugging run so could be removed but if you are still using them feel free to leave
| self.pdb_dir = pdb_dir | ||
| self.pdb_prefix = pdb_prefix if pdb_prefix is not None else "fixed_" | ||
| self.pdb_ext = pdb_ext or ".pdb" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the changes in this file would break some of the current training code - could you adapt your training code to use these args and uncomment the code below
| # calculate the L1 divergence of the SAXS profiles | ||
| l1_loss = nn.L1Loss(reduction="sum") | ||
| loss = l1_loss(pred_saxs, true_saxs) | ||
| loss = l1_loss(pred_saxs, saxs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you revert these changes? I don't think saxs should be an input anymore in this function
| "openfold @ git+https://github.com/aqlaboratory/openfold.git@103d037", | ||
| "ml-collections==0.1.0", | ||
| "dm-tree==0.1.6", | ||
| "lightning==2.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "lightning==2.1.0", |
This is already specified above
Hi Steph,
This is my random sample branch with the ensemble generation ability. Could you have a look at this? I checked and there is not much overlap or conflicts with the main branch.
Best
Feng Yu