allow custom shape points by greglandrum · Pull Request #8799 · rdkit/rdkit (original) (raw)
Exposes an API to allow the user to set custom feature types for the shape alignment code.
Inspired by some work done together with @brje01
| if (shapeOpts.customFeatures.empty()) { |
|---|
| extractFeatureCoords(conformer, nAtoms, nSelectedAtoms, feature_idx_type, |
| shapeOpts, ave, numFeatures, res, rad_vector); |
| } else { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When do you re-size res.coord to accommodate the custom feature coords? nAlignmentAtoms is nAtoms + feature_idx_type.size() and I think feature_idx_type doesn't have the custom features in it? Unless I missed something. Maybe it should be nAtoms + feature_idx_type.size() + shapeOpts.CustomFeatures.size() and that would cover all bases.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, very good catch.
I'm surprised valgrind didn't signal about that.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments. I think at least the res.coord resizing thing needs checking before merge.
| \param max_postiters (optional) the max number of post-optimization |
|---|
| iterations |
| \return a pair of the shape Tanimoto value and the color Tanimoto value (zero |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function doesn't have a useColors parameter. I think that's sensible, as there's also one in ShapeOpts, so which one does it use? But it is inconsistent with the other functions and makes the remark about the return value a bit less clear. How does useColors interact with opt_param? The latter seems to make the former redundant. And what about the ShapeInputOptions.useColors. It's maybe outside the scope of this PR, but there's a lot of potential confusion over when colours are used or not.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The useColors argument just determines whether or not color points are added to the shapes constructed from the molecules. In the new functions, that can be applied individually to the reference and the fit molecule (of course it doesn't make sense to have useColors set for one and not the other, but we can't do everything, I added a warning for this case).
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
[ Show hidden characters]({{ revealButtonHref }})