Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #882 +/- ##
==========================================
+ Coverage 60.37% 60.40% +0.03%
==========================================
Files 103 103
Lines 31153 31156 +3
Branches 8120 8121 +1
==========================================
+ Hits 18808 18821 +13
+ Misses 9996 9989 -7
+ Partials 2349 2346 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
samcogan21
left a comment
samcogan21
left a comment
There was a problem hiding this comment.
Looks good! Added two suggestions.
| if self.multiplicity is None: | ||
| self.multiplicity = self.mol.multiplicity | ||
| if self.is_ts and self.get_xyz(generate=False): | ||
| self.determine_multiplicity_from_xyz() |
There was a problem hiding this comment.
Might be worth adding a log line here- so it will be easier to debug if there's a multiplicity issue that still arises.
Something like:
logger.debug(f"TS species {self.label}: using xyz-based multiplicity {self.multiplicity} (ignored mol.multiplicity)") would make debugging much easier down the road, especially since a wrong multiplicity here causes Gaussian to reject the job entirely.
|
|
||
| ts_1_spc_from_dict = ARCSpecies(species_dict={'label': 'TS1', 'is_ts': True, 'xyz': ts_1_xyz}) | ||
| self.assertEqual(ts_1_spc_from_dict.multiplicity, 1) | ||
|
|
There was a problem hiding this comment.
Can we add a known TS with a multiplicity of 2 (actual radical) to make sure the XYZ doesn't flatten it to 1?
|
Thanks, @samcogan21! fixed and squashed |
2 participants
Issue
A TS species defined in input.yml with only is_ts: true and xyz (no smiles/adjlist) could be assigned the wrong multiplicity, causing Gaussian to reject the input with GL301. For example, a closed-shell TS with 24 electrons was written as 0 2 (mult=2), which is electronically impossible.
Root cause
ARCSpecies.from_dict (the path taken when main.py loads species from YAML via ARCSpecies(species_dict=spc)) called mol_from_xyz to perceive a 2D graph from the TS geometry, then blindly assigned self.multiplicity = self.mol.multiplicity. RMG's perception of TS-like geometries (partial bonds, near-radical atoms) routinely returns spurious biradical structures — for the failing case it returned O=[C][H][OH] with mult=2. Direct construction (ARCSpecies(label=..., is_ts=True, xyz=...)) was unaffected because init routes through determine_multiplicity, which skips the descriptor path for TSs and uses electron parity from xyz.
Fix
In from_dict, when is_ts=True and xyz is available, fall back to determine_multiplicity_from_xyz (electron-parity) instead of trusting the perceived mol.multiplicity. Non-TS behavior is unchanged; TSs without xyz still fall through to mol.multiplicity since there's no parity signal.
Test
Added a dict-construction assertion to test_determine_multiplicity for the failing TS xyz — this is the path the bug actually went through. The pre-existing direct-construction assertion was insufficient because it bypassed from_dict entirely.