-
Notifications
You must be signed in to change notification settings - Fork 252
Description
In checkForExistingReaction(self, rxn), rxn.reactant.sort() is firstly implemented to created unique order of multiple reactants. But this sort() doesn't provide any sorting key. After some testing, I guess sort() method if no sorting key provided, is sorting based on object's memory address (it's only a hypothesis, anyone who is familiar with it please let us know.), which makes the order of reactants in a same reaction non-deterministic if you run the same RMG job multiple times (I've already observed this phonomenon).
But the reason why currently RMG still can produce same final mechanism is that we only allow one species instance to exist in memory for one species.
The potential problem of this sort() implementation is when unpickling restart files, the memory address of loaded reactants will change so the order of reactants in reactions should change accoordingly, if not, some existing reactions cannot be recognized further leading to multiple DUPLICATE reactions in rmg.reactionModel.reactionDict.
I suggestion this sort() should be provided by some appropriate sorting key to make it more deterministic. I've tried species.index since every reactive species will have a unique index at this time point (when implementing checkForExistingReaction(self, rxn)), which can now solve the non-deterministic problem. But more thoughts about sorting key are welcome.