Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent dot plot/pairing probabilities behavior between Vienna/Vienna2 and everything else #803

Open
luxaritas opened this issue Aug 16, 2024 · 0 comments
Labels
priority: p2/important ⚠️ A large number of users have a significant pain point or a significant use case is prevented size: md type: bug Something isn't working

Comments

@luxaritas
Copy link
Member

Two issues here:

  1. Vienna/Vienna2 dot plot probabilities are sqrt of the actual values (see Vienna[2] dot plot probabilities are sqrt(true prob) #291). This leads to extra work in EternaJS to make sure we square when we need to, but also liable to cause confusion for eternascript authors
  2. Vienna/Vienna2 include the target structure, but other engines do not. This means that the dot plot has an empty lower triangle for most engines, but also this is a point of confusion for script authors, who (particularly given the current documentation and naming) would expect the output of the pairing_probabilities API to not include the "lower triangle". There also appears to be some cases in our code (eg for expectedAccuracy, pUnp, etc) where we also don't do this correctly!

The issue with just fixing it outright is that by this point, a number of eternascript authors have implemented workarounds in their own scripts, which means that scripts that currently behave correctly because of a custom patch (or other reliance on the current behavior) will then behave incorrectly - and script authors who do this may not realize it has been fixed, so continue to rely on the incorrect behavior

We've decided to fix this by:

  1. Renaming pairing_probabilities to pairing_probabilities_incorrect, which will continue to have the current behavior.
  2. In all existing eternascripts, replace calls to pairing_probabilities with pairing_probabilities_incorrect
  3. Introduce a new corrected function. This should have a different name, so that if an existing script author goes to use pairing_probabilities with the expectation of the old behavior, they find it no longer exists, looks at the docs, and sees there's a new function that behaves differently. I propose calling this base_pairing_probabilities.
@luxaritas luxaritas added type: bug Something isn't working priority: p2/important ⚠️ A large number of users have a significant pain point or a significant use case is prevented size: md labels Aug 16, 2024
@github-project-automation github-project-automation bot moved this to Todo in Roadmap Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2/important ⚠️ A large number of users have a significant pain point or a significant use case is prevented size: md type: bug Something isn't working
Projects
Status: Todo
Development

No branches or pull requests

1 participant