Repository navigation
Change residue level MOI calculation to all atom - #445
Conversation
harryswift01
left a comment
There was a problem hiding this comment.
Hi @ioanaapapa , thanks for this, this is looking like a great PR. Moving the residue-level MOI calculation to an all-atom approach makes CodeEntropy more consistent, and I really appreciate you updating the regression baselines alongside it. I've spotted a few minor things and left comments for each: removing get_UA_masses and its related tests as it's no longer used, removing the unused box argument from get_residue_axes_from_topology, and adding a unit test for the bonded residue path to check the all-atom positions and masses are being used. If you're able to have a look and let me know what you think, that would be great, and if there's anything you'd like me to help with or clarify, please just ask. Overall this is looking very good and is close to being merged!
|
@harryswift01 Thanks, Harry! I'll make the changes. |
… axes used elsewhere
…lts after conformational entropy bug fix
|
Hi @harryswift01. I've made the changes, hopefully it's all ok now. |
harryswift01
left a comment
There was a problem hiding this comment.
Thank you for applying all of my suggested changes to this PR @ioanaapapa! This is all looking good to me so I'm happy to see this merged in!
Summary
This PR closes #441 and changes the MOI calculation at the residue level to use all atoms masses and positions, rather than heavy atom positions and UA (heavy atom + hydrogens) masses. This corresponds to what we discussed in the CodeEntropy catch up meeting on 8 October 2026.
Changes
Impact
benzene_MOIs.txt
thioredoxin.txt