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

data sharing bug in EN_increasing_depth #200

Closed
bkatiemills opened this issue Aug 13, 2017 · 0 comments
Closed

data sharing bug in EN_increasing_depth #200

bkatiemills opened this issue Aug 13, 2017 · 0 comments

Comments

@bkatiemills
Copy link
Member

bkatiemills commented Aug 13, 2017

There is a bug in how EN_increasing_depth is sharing data between runs, on master (ie the postgres-and-global-variable-backed implementation). To reproduce:

  • dataset consisted of 10 profiles:
563906
61025844
61033936
86275202
86275203
86275204
86275205
86275206
86275207
88253961
  • running autoqc as normal flags 88253961 as bad by en_increasing_depth
  • turning off EN_std_lev makes 88253961 report as good by increasing.

The cause is in EN_increasing_depth, which recall is used in EN_std_lev. increasing nominally stores the uid and qc of the last profile it analyzed in its corresponding global variables. But, in the clause considering where all depths are the same, the qc global is updated while the uid global is not, creating a pathology in the above series of profiles:

  • 61033936 considers 88253961 as a buddy profile; 88253961's uid and qc results are now in the globals of en_increasing_depth_check (everything normal so far)
  • all the profiles 8627520- have only one level, therefore guaranteed to enter increasing's 'all levels are the same depth' clause. increasing's globals now contain the qc result for the 8627520- series, but still uid 88253961
  • autoqc reaches 88253961, believes it has a qc result for it in increasing's globals, and incorrectly uses it.

This bug does not currently appear in no-global branch, since using sqlite as a backing rather than global variables removed the effect of the 8627520- series profiles on the unrelated 88253961, thus explaining at least one part of the regression failures mentioned by @s-good in #195.

A pr to master correcting this will be along shortly.

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

No branches or pull requests

1 participant