-
Notifications
You must be signed in to change notification settings - Fork 12
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
Reorder atoms in label data (Fixes 502) + documenting C++ #521
Reorder atoms in label data (Fixes 502) + documenting C++ #521
Conversation
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 see that canonicalization was used for the re-ordering at dataloading and data preparation time. The only issue is that we can't simply use the "predict mode" for real-world predictions without un-ordering. But I think it's fine. We should probably warn about that in the docs and the code
- Log warning about canonicalization cannot be used for predictions without un-ordering
- Warn in the documentation about the point above
1dfbf12
to
3f9dcb6
Compare
…ame molecule appears in multiple tasks with different atom orders, to be consistent with the first task's atom order
…lude merge_equivalent_mols parameter
…during pre-processing
…gs, compute_mol_keys, compute_stats, save_non_label_data, and save_label_data, to make the code more manageable.
…sks with the same molecule and one of them has edge-level label data. This should order bonds consistent with the first copy of the molecule.
… atom classes in the SMILES strings, and only if they represent a complete atom order, to fix the accidental removal of support for explcit ordering. The atom classes (atom map numbers) are then removed, so that they don't cause equivalent molecules to have different canonical atom orders. This allows the ordered parameter to default to true again. Some callers may want to set this to false, for performance.
…ses in SMILES strings, not 1-based
…_laplacian_eigendecomp to accept `const int32_t*` instead of `const std::vector<int32_t>*`
3f9dcb6
to
5d798a5
Compare
Changelogs
Checklist:
feature
,fix
ortest
(or ask a maintainer to do it for you).discussion related to that PR