Repository navigation
optimizer: report failed structures as None instead of dropping them - #41
Merged
niklashoelter merged 1 commit intoSep 25, 2026
Conversation
BREAKING: optimize_structure_batch now returns list[Structure | None].
_optimize_sequential appended only the structures that optimized, so the
returned list was shorter than the input and every result after a failure sat
at the wrong index, with nothing to say so. Given inputs [A, B, C, D] where C
fails, the caller received [A, B, D] -- and api.py, which labels output by
position, then wrote D into the file as "Optimized structure 3".
Return one entry per input, in input order, with None marking a failure. This
matches embed.generate_structures, which already reports per-molecule failure
this way, so the two halves of the package now agree.
Callers that iterate results must skip None. Inside the package:
- log_optimization_summary filters them out, so the success rate counts what
actually came back rather than counting a None as an output.
- The three api.py entry points still return list[Structure], filtered, so
the high-level API is unchanged. What changes there is the labelling:
comments are numbered by position in the *input*, so a failed structure
leaves a gap instead of renumbering everything after it.
The batch path never had the defect -- torch-sim's restore_original_order
keeps input order and raises on a count mismatch -- but its annotation moves
to the shared return type.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BREAKING: optimize_structure_batch now returns list[Structure | None].
_optimize_sequentialappended only the structures that optimized, so on afailure the returned list was shorter than the input and every later result sat
at the wrong index.
api.pylabels output by position, so for inputs[A, B, C, D]withCfailing,Dwas written as "Optimized structure 3".Now one entry per input, in input order, with
Nonemarking a failure. Thismatches
embed.generate_structures, which already reports per-molecule failurethis way.
Breaking:
optimize_structure_batchreturnslist[Structure | None].Callers iterating the result must skip
None. The threeapi.pyentry pointsstill return a filtered
list[Structure], so the high-level API is unchanged;only their comment numbering changes, leaving a gap at a failed input instead
of renumbering.
log_optimization_summaryfiltersNoneso the success ratecounts real outputs. The batch path never had the defect (torch-sim keeps input
order and raises on a count mismatch); only its annotation moves.
Tests: 4 new in
tests/optimization/test_batch.py. Full suite: 248 passed,3 skipped.