Repository navigation
Conversation
2619e53 to
5f6a9b3
Compare
| # adapter, so neither a human reading the log nor a caller iterating over maps | ||
| # has to infer it from a guess count. | ||
| self.atom_maps_without_guesses.append(list(forced_atom_map)) | ||
| rxn.ts_species.append_ts_guess(TSGuess(method=f'Linear (map={map_i})', |
There was a problem hiding this comment.
a failed map can stop the remaining maps working. so if we save a failed entry without coords, then the next map produces a geom it tries to compare the geom with the missing coords and crashes. shuold also have a test bad map, then good map
There was a problem hiding this comment.
worse than a crash: the TypeError from almost_equal_coords is swallowed by the except Exception around the reaction, so the whole reaction was abandoned, not just that map. [bad, good] returned 0 guesses; it now returns 3. Coordinate-less guesses are skipped during deduplication, and the bad-map-then-good-map test you asked for is in.
| comment=f'Linear w={w:.2f}, {xyz_i}{map_suffix}, family: {rxn.family}', | ||
| ) | ||
|
|
||
| if forced_atom_map is not None and len(rxn.ts_species.ts_guesses) == guesses_before: |
There was a problem hiding this comment.
a valid duplicate guess is called a failure. if a map produces a geom already found earlier, obv it will avoid saving it twice but it then interprets nothing new was saved as "this map failed"
There was a problem hiding this comment.
right, and there were two causes: the append-time deduplication you spotted, plus interpolate suppressing against the shared existing_xyzs before returning, which made "all duplicates" indistinguishable from "produced nothing". Deduplication is now per map, and a map is judged on geometries produced rather than guesses appended. Guesses are still deduplicated.
|
|
||
| if len(self.reactions) < 5: | ||
| successes = len([tsg for tsg in rxn.ts_species.ts_guesses if tsg.success and 'linear' in tsg.method.lower()]) | ||
| requested = len(self.forced_atom_maps) if self.forced_atom_maps is not None else 0 |
There was a problem hiding this comment.
when reporting results for the second reaction, the code also counts failures from the first reaction. this can produce results such as "-1 out of 1 requested maps succeeded"
There was a problem hiding this comment.
reproduced your exact -1 of 1. The comment in that code claimed it counted only the current reaction, which was false: it restricted to the requested set, not the reaction. Now a per-reaction delta; both reactions report 0 of 1.
calvinp0
left a comment
calvinp0
left a comment
There was a problem hiding this comment.
thanks mate! just a few comments
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## linear_adapter_explicit_atom_map #1046 +/- ##
====================================================================
- Coverage 67.09% 67.07% -0.02%
====================================================================
Files 123 123
Lines 43741 43754 +13
Branches 11170 11172 +2
====================================================================
+ Hits 29347 29348 +1
- Misses 11236 11245 +9
- Partials 3158 3161 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
5f6a9b3 to
1a3c192
Compare
This branch has not been deployed
2 participants
Summary
A requested atom map for which no TS guess survives validation is now a distinct outcome instead of a smaller result set: it is appended to
LinearAdapter.atom_maps_without_guesses, recorded on the reaction as an unsuccessfulTSGuess(so it persists into the restart file), and logged as a warning, with the summary line reporting how many of the requested maps produced guesses.Investigating the reported case: nothing raises -
interpolate_isomerizationreturns empty, every candidate having been discarded by the H-migration validators. Near-identity maps are not the trigger; over the 7 enumerated maps of[CH2]C(C)CC -> CC(C)[CH]Cthe most identity-like map (11/16 fixed points) does produce a guess. The two maps that matter share an identical reaction center (formed(0, 12), broken(3, 12)) and differ only in the permutation of the spectator hydrogens, yet one yields 3 guesses and the other none - the ordered-product geometry is built by reordering P through the map, so spectator labeling changes the Z-matrix blend. That sensitivity is a property of the interpolation path rather than of the explicit-map argument, and is left for a separate change; the warning notes that another map with the same reaction center may still produce a guess.Based on #1044.
Closes #1045