Atlassian uses cookies to improve your browsing experience, perform analytics and research, and conduct advertising. Accept all cookies to indicate that you agree to our use of cookies on your device. Atlassian cookies and tracking notice, (opens new window)
A mix of the above and a new FrozenMolecule subclass that does not yet exist
Multiple export destinations with different requirements
Case where duplication is benign
Case where failing to deduplicate is fatal
Cases “identical” mols may have different physics params.
MT ideas
Have Molecule.are_isomorphic take in 3 or more types (Molecule, _SimpleMolecule, nx.Graph, other FrozenMolecule subclasses)
Make _SimpleMolecule.are_isomorphic and then when an isomorphism check is needed, look at the types that are to be compared and either call Molecule.are_isomorphic
Have Molecule.are_isomorphic not know about its inputs, but immediately convert its inputs to graphs via getattr(mol1, "to_networkx")() and error out if that conversion fails.
JW ideas
Different classes can never be compared (always returns false)
There's a hierarchy of subclasses where one is always identifiable as "more specific' and will be used for isomorphism checking
isomorphism checks accept a lambda function, which can change how the comparison is done to allow for use in different contexts
MT – In favor of “Molecule” compared to “SimpleMol” always fails.
JW – In favor of “everything turns itself into a networkx graph”, paired “pass in lambda functions describing how you define identity”
MT – Worth thinking about timelines - I want to get this out ASAP, generalized handling is nice but it’s not clear that people need this soon.
PAths forward
Option 1
NOW: Different types are never isomorphic, we implement SimpleMol.are_isomorphioc, objects of the same type can compare to each other
LATER: either objects of different types know how to compare to each other, or
Option 2
Long-run decisions
Do objects carry around their own comparison logic, or is comparison logic provided externally?
If they carry around their own logic, we get into molecule subclass hell, where mols need to know what they’re exporting to to behave right, even before they’ve been exported.
So I think the identity-defining logic should not be in the Molecule class
To keep with the old API, we need to keep Molecule.are_isomorphic. We could add SimpleMolecule.are_isomorphic but it would be short-lived.
Where does the short circuit logic live?
Call path could go
Topology.custom_grouping(IsomorphismChecker)
→ short circuit logic
→ IsomorphismChecker(obj1, obj2)
→ short circuit logic
→ Either
obj1.is_isomorphic_with(obj2)
custom logic, whatever IsomorphismChecker wants.
Galaxy brain idea
Short circuits are their own objects and know when they’re applicable and when they’re not
Short-term:
MT want a way to use Topology.identical_molecule_groups to work on a topology that is ONLY _SimpleMolecule
This could be implemented by
Add _SimpleMolecule.are_isomorphic
Inside of the Topology method, compare types:
If they are both Molecule, use existing code path
If the are both _SimpleMolecule, call the new _SimpleMolecule.are_isomorphic
If they are one and the other, return False
Inside (simple)Molecule.are_isomorphic
return False if other is an unsupported type
Tests involve:
Create a Topology of _SimpleMolecule and make sure that Topology.identical_molecule_groups behaves as expected
Make sure _SimpleMolecule.are_isomorphic does what we expect
Given a mixed topology, make sure checks from either class appropriately short-circuit
Action items
Decisions
, multiple selections available, Use left or right arrow keys to navigate selected items