Drop pose-variant absolute coordinates from native-3d descriptors - #112
Merged
Conversation
The native-3d preset emitted centroid_x/y/z and center_of_mass_x/y/z: absolute coordinates in the input file's frame, so translation- and rotation-variant. Two identical molecules placed differently produced different features, letting a model learn the arbitrary input frame -- a footgun in a table whose purpose is fixed-size ML features. Size and shape are already captured frame-independently by radius_of_gyration, dim_x/y/z, principal_moments and the gyration-tensor shape scalars. Remove all six from the emission, the empty-structure path, and the native-3d name list, so preset=None and preset="native-3d" stay consistent (width 80 -> 74). Nothing internal, tested or documented depended on them. native-basic and rdkit-basic are unaffected. Breaking change to the native-3d schema; noted in CHANGELOG. A regression test pins the columns' absence.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the descriptors review. The
native-3dpreset emitted six absolute-coordinate columns —centroid_x/y/zandcenter_of_mass_x/y/z— recorded in the input file's coordinate frame. They are translation- and rotation-variant: two identical molecules placed differently in their files get different feature values, letting a model learn the arbitrary input frame. In a table whose stated purpose is fixed-size ML features, that is a footgun. Size and shape are already captured frame-independently byradius_of_gyration,dim_x/y/z,principal_moments, and the gyration-tensor shape scalars.Changes
native-3dname list — sopreset=Noneandpreset="native-3d"stay consistent. Width drops 80 → 74.native-basicandrdkit-basicare unaffected.descriptor_feature_names, and the defaultpreset=Noneoutput).Verification
This came out of an empirical review of the shape descriptors. I also checked the related claim that
asphericity/acylindricityare redundant withrelative_shape_anisotropy— across 23 shape-diverse molecules, leave-one-out R² against the principal-moments block was 0.65 / 0.93 / negative respectively (all below the 0.96 redundancy bar, and mutually uncorrelated), so those were kept. Only the pose-variant coordinates are removed here.Note
This is a breaking change to the
native-3dschema. It is recorded under[Unreleased]; it warrants a 0.15.0 → 0.16.0 bump whenever the next release is cut (left out of this PR).Full suite: 867 passed, 21 skipped.
ruff checkclean.