fix three bugs in dynamics/analysis.py (dropped hinges, IndexError, wrong collectivity norm)

[steps-re]

disclosing up front: this was put together with help from claude (anthropic's coding agent), then reproduced and verified by me before opening. first pr i'm opening on prody.

three separate bugs in prody/dynamics/analysis.py, all found while looking at the hinge-finding code. each is small and independent so happy to split into separate prs if you'd rather.

1. getGlobalHinges silently drops every chain's hinges but the last

the if trim is not False / else block that accumulates hinges is indented at the same level as for fst, lst in chains:, so it runs once after the chain loop finishes, using only the leftover h/l/fst from the last chain. for any multi-chain structure, every earlier chain's hinges are silently thrown away. fix is to move the accumulation inside the loop.

there's also an off-by-one in the same spot: chains stores inclusive (fst, lst) pairs, but the code slices vecs[fst:lst, i] (exclusive), which drops each chain's last residue from hinge detection. fixed to vecs[fst:lst+1, i].

2. getHinges crashes on a segment with no sign change

merged = [regs[0]] raises IndexError when regs is empty, which happens whenever an eigenvector segment has no sign crossing (realistic for short segments, and more likely once bug #1 is splitting by chain). added a guard to return no hinges instead of crashing.

3. calcCollectivity mis-normalizes when masses are supplied

Bruschweiler 1995 needs the per-atom msd normalized into a probability distribution (p_i = u_i^2 / sum(u_j^2)) before the shannon-entropy step. the code divides by sqrt(sum(u_j^2)) instead. this cancels on the default path because mode vectors are already unit-normalized (sum == sqrt(sum) == 1), so it only shows up when masses is passed, which is the documented-supported path. for v=[0.6,0.8], masses=[1,4] the current result is ~0.9878 vs the correct ~0.9269 (~35% off). fix is to divide by the sum, not its sqrt.

verification

added prody/tests/dynamics/test_analysis_hinge_bugs.py (offline, deterministic, no downloads): 4 tests targeting the bugs + 2 control/regression guards. confirmed all 4 fail on current main (IndexError, dropped-chain hinge mismatch, wrong collectivity value) and pass after the fixes. ran the full prody/tests/dynamics/ suite, no regressions (existing test_hinge_finder.py still passes). the default no-masses collectivity path is unchanged.

mike

[stoneandbone]

Hi @steps-re , thank you very much for reviewing the code and submitting a PR. Yes, point 1-2 are both valid. I meant to push these changes earlier, along other changes. But thank you for catching them.

As to Bruschweiler 1995 eq. 5-6., I do see your point about proper representation of the probability function, which is required to equal to 1 in order to compute the associated entropy. But with the square root, it does not. Just to have another pairs of eyes to confirm if @jamesmkrieger or @AnthonyBogetti can take a quick look, that would be great!

[jamesmkrieger]

Looks good to me

Give the tests file a more generic name like test_hinges

Also double check that the collectivity fix doesn’t break the no masses case

[steps-re]

renamed it to test_hinges.py.

on the no-masses case: it comes out identical, and the reason is that the normalization line sits outside the masses block so it runs on both paths. for real modes sum(u2) is already 1, so the old 1/sum**0.5 and the new 1/sum are the same number. checked 1ubi GNM and ANM, first 5 modes each, old vs new agree to 0 or 1e-17:

GNM mode 0: sum(u2)=1.0000000000000004  old=0.10174   new=0.10174
ANM mode 3: sum(u2)=0.9999999999999997  old=0.039629  new=0.039629

the two only diverge if someone passes a raw unnormalized array, which the V branch at the top does allow. there the old form was scale-dependent and the new one is not:

sum(u2)=1.0   old=0.70568  new=0.70568
sum(u2)=2.0   old=0.78588  new=0.70568
sum(u2)=0.5   old=0.59083  new=0.70568
sum(u2)=10.0  old=0.41946  new=0.70568

old normalizes to 1.414 and 0.707 in those rows rather than 1, so it was not a probability distribution and the entropy term was off. collectivity should not depend on how the input array is scaled, so i think new is the behavior you want, but flagging it since it is a real behavior change for that input.

prody/tests/dynamics/ is 165 passed. the 5 test_anmd.py failures are pre-existing, No module named 'simtk', same 5 fail on the base commit.

[jamesmkrieger]

great.

Probably the test_anmd.py failures are from importing openmm in a wrong way or having a wrong version of it installed. I'll have a look at it

[jamesmkrieger]

There's already a file test_hinge_finder.py, so probably your new hinge tests should go there. We could do with a general prody/tests/dynamics/test_dynamics_analysis.py really though for general things like collectivity

[jamesmkrieger]

ok, the main test_anmd problem is that openmm imports were before type checks. There were also a few others, so I am making a branch and PR to fix these

[steps-re]

moved them. the three hinge classes are in test_hinge_finder.py now next to the GNM ones, and collectivity got its own test_dynamics_analysis.py like you suggested. test_hinges.py is gone.

test bodies are unchanged. i ran them against the pre-fix analysis.py and the same four still fail there and pass after, so the move didn't quietly defang anything.

prody/tests/dynamics is 155 passed with the 5 test_anmd simtk import failures you're already on, and those show up without my changes too.

[jamesmkrieger]

Great, ready to merge