Skip to content

Fix residue conformational state indexing by group id - #459

Merged
harryswift01 merged 3 commits into
mainfrom
368-bug-investigate-ambiguous-residue-conformational-state-indexing
Oct 14, 2026
Merged

harryswift01 merged 3 commits into
mainfrom
368-bug-investigate-ambiguous-residue-conformational-state-indexing

Conversation

@harryswift01

Copy link
Copy Markdown
Member

Summary

This PR fixes residue-level conformational entropy being reported as 0.0. Residue states were stored in a list that was pre-filled with one empty list per group and then appended to, so lookups by group id read the empty placeholders. The residue states and flexible-dihedral counts are now keyed by group id, and the regression baselines affected by the correction are regenerated.

Changes

Key residue conformational state accumulators by group id:

  • states_res and flexible_res in conformational_state_builder.py are now dict[int, ...] instead of lists, and the return annotations are updated to match.
  • _merge_group_state_data in state_assignment.py takes group_id and assigns states_res[group_id] and flexible_res[group_id] instead of appending.
  • Removed the now-unused number_groups.

Update unit tests for group-keyed states:

  • Builder tests (no groups, empty group, chunked serial flow) now expect dicts.
  • The chunked serial test previously asserted [[], []] for a single group, which encoded the indexing bug.
  • The merge test passes a non-zero group_id to check the key is used and not the insertion order.

Update regression baselines:

  • Regenerated the baselines for all systems affected by the corrected residue-level conformational entropy.

Impact

  • Residue-level conformational entropy changes from 0.0 to a computed value for any run with a residue level. Results will differ from earlier versions.
  • On the DNA system from [Bug]: Investigate ambiguous residue conformational state indexing #368 (all atoms, customised_axes: false, frames 0-5 step 2), group 0 residue Conformational goes from 0.0 to 5.292271495278918, matching the issue.
  • flexible_res is no longer indexed by position, so skipped or non-contiguous groups can't read another group's value. A missing group now raises KeyError.

@harryswift01 harryswift01 self-assigned this Oct 14, 2026
@harryswift01 harryswift01 added the bug Something isn't working label Oct 14, 2026
@harryswift01 harryswift01 linked an issue Oct 14, 2026 that may be closed by this pull request
@harryswift01 harryswift01 added this to the 2.6.0 milestone Oct 14, 2026

@jimboid jimboid 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.

Looks like this fixes a real bug behind at least one reported issue. Happy to merge.

@skfegan skfegan 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.

This looks good. Hopefully the tests ensure that this issue doesn't recur.

@harryswift01
harryswift01 merged commit 77ae581 into main Oct 14, 2026
23 checks passed
@harryswift01
harryswift01 deleted the 368-bug-investigate-ambiguous-residue-conformational-state-indexing branch October 14, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Investigate ambiguous residue conformational state indexing

3 participants