This actually resolves a couple issues:
1) A crash when using the t-loop stamper
2) Potentially unknown bugs when using the t-loop stamper due to a partially corrupted internal structure representation
3) When using either the t-loop stamper or target structure paste, there is a chance you could violate structure constraints (when pairing bases a and b, we never checked if b is paired to c, that c is able to be made unpaired)
Implementation Notes
Speaking to (1)/(2): What happened here is that the t-loop stamper was only configured to set pairs from 5' to 3' and not 3' to 5' (eg, in either stamp, we specified that the first base should be paired to the last base, but not that the last base should be paired to the first base). The crash occured if you stamped over an existing target structure such that whatever the second base was previously paired to was never unpaired. But also it meant that the internal pairmap was not symmetric, which could lead to some other issue later down the line.
Ultimately this was due to the fact that SecStruct#setPairingPartner was meant to be called twice, once in each direction. Presumably this was either an oversight or a performance optimization. That said, this has already bitten me before (see 0bf6132b4248859051b00021e5c19595d84a3e9f), and is not intuitive behavior. So I've updated this function to do the reasonable thing and just handle both directions by default. The performance penalty shouldn't be meaningful.
This inadvertently helped me realize (3) was an issue.
Summary
This actually resolves a couple issues: 1) A crash when using the t-loop stamper 2) Potentially unknown bugs when using the t-loop stamper due to a partially corrupted internal structure representation 3) When using either the t-loop stamper or target structure paste, there is a chance you could violate structure constraints (when pairing bases a and b, we never checked if b is paired to c, that c is able to be made unpaired)
Implementation Notes
Speaking to (1)/(2): What happened here is that the t-loop stamper was only configured to set pairs from 5' to 3' and not 3' to 5' (eg, in either stamp, we specified that the first base should be paired to the last base, but not that the last base should be paired to the first base). The crash occured if you stamped over an existing target structure such that whatever the second base was previously paired to was never unpaired. But also it meant that the internal pairmap was not symmetric, which could lead to some other issue later down the line.
Ultimately this was due to the fact that
SecStruct#setPairingPartnerwas meant to be called twice, once in each direction. Presumably this was either an oversight or a performance optimization. That said, this has already bitten me before (see 0bf6132b4248859051b00021e5c19595d84a3e9f), and is not intuitive behavior. So I've updated this function to do the reasonable thing and just handle both directions by default. The performance penalty shouldn't be meaningful.This inadvertently helped me realize (3) was an issue.