Skip to content

Phase 1: audit and regression-backed correctness fixes - #233

Open
Alexander-Mitrofanov wants to merge 27 commits into
masterfrom
refactoring
Open

Phase 1: audit and regression-backed correctness fixes#233
Alexander-Mitrofanov wants to merge 27 commits into
masterfrom
refactoring

Conversation

@Alexander-Mitrofanov

@Alexander-Mitrofanov Alexander-Mitrofanov commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Implements phase 1 of discussion #232.

  • documents the current architecture, scientific invariants, bottlenecks, and remaining risks in doc/refactor/1-current-state.md
  • adds regressions before fixes, including ensemble algebra/filtering, seed reuse, boundary, accessibility, and CLI option isolation cases
  • keeps each local bugfix in an independent commit
  • preserves intentional centi-kcal, coordinate, and tie semantics

Validation:

  • optimized build passes
  • API: 3,934 assertions in 34 test cases
  • CLI: all 20 golden/error scenarios pass
  • full make tests: 2/2 harnesses pass

Record the 3.4.1 architecture, numeric contracts, bottlenecks, test gaps, and independently reproduced correctness findings for discussion #232. Add focused regressions before fixes: Interaction self-assignment and null-seed equality, zero-capacity result storage, invalid Nussinov paired intervals, base-pair ES semantics, noLP/terminal-GU partition counting, and target/query accessibility option isolation. These regressions intentionally fail on the audited baseline so each correction can follow independently.
Return before clearing owned pairing and seed state when the source and destination are the same Interaction. This makes the assignment contract safe without changing normal copy behavior.
Treat identical seed pointers as equal and compare seed sets only when both interactions own one. Seeded and unseeded interactions now compare unequal instead of dereferencing null.
Continue counting reported candidates when maxToStore is zero, but skip list comparison and insertion. This avoids dereferencing an empty reverse iterator for --outNumber=0 style consumers.
An out-of-range paired state is impossible and must contribute the additive zero of the partition semiring. Returning one incorrectly admitted an invalid pair as a multiplicative identity.
getES1/getES2 promise the ensemble of intramolecular structures containing at least one pair. Subtract the unit-weight empty structure before converting Q to energy, while leaving getEall1/getEall2 based on the complete monomer partition.
The direct continuation after the mandatory left stack already covers the adjacent (1,1) extension. Skip that one loop transition so the heuristic remains a subset of the exact interaction ensemble instead of assigning duplicate weight.
Reject terminal-GU and excessive-ED sites in updateZ before they alter Zall or the boundary partition. Reported candidates and their requested interaction ensemble now use the same scientific domain.
Construct target AccessibilityBasePair with tIntLenMax and tAccW. Query settings must not truncate the target ED matrix; asymmetric target/query CLI options now retain their independent biological domains.
@martin-raden

Copy link
Copy Markdown
Member

Hi Alex,
that looks great! Many thanks for this and this long list of bugfixes!

For merging, could you please trigger to update the Changelog file accordingly, to prepare the bugfix release in the current version 3* branch.

Given that, I will do some local checks and merge it to master!

👍👍
Supercool, thanks a lot!
Best,
Martin

@martin-raden

Copy link
Copy Markdown
Member

PS. I already reviewed all code base changes and had a close read of the refactoring plan.

@martin-raden martin-raden left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have checked and approved all changes.

  • TODO: update "Changelog" file accordingly

and this will be ready to merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants