Refactory zygosity logic #430
Comments
|
Let’s discuss on the next call. (I’m not here this week though.)
From: Cyrille Rossant <notifications@github.com>
Sent: 08 August 2018 09:09
To: cortex-lab/alyx <alyx@noreply.github.com>
Cc: Harris, Kenneth <kenneth.harris@ucl.ac.uk>; Mention <mention@noreply.github.com>
Subject: [cortex-lab/alyx] Refactory zygosity logic (#430)
Rules are currently hard-coded ; they should be part of the database definition instead. We may need to revisit the schemas.
This is related to a bug reported by @charureddy<https://github.com/charureddy>, the correction of which might be the occasion to rethink the way it's currently implemented.
cc @kdharris101<https://github.com/kdharris101> @nsteinme<https://github.com/nsteinme>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#430>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AD7KlTnmGUO8JccN-sDyWRxGy6NaUm0vks5uOuMFgaJpZM4Vz5ty>.
|
|
What we'll need to define clearly is the algorithm specifying (1) when the zygosities are set and (2) how they are set, depending on the line, the sequences, the genotype tests, the parents genotypes, including when the parents are from different lines. It seems this algorithm is not clearly defined right now and the code is behaving incorrectly in some cases. |
|
Thanks Cyrille. We have to resolve this soon, as currently, users have no
idea whether an animal is negative or positive, and this can end up in them
picking the wrong animal.
Regards,
Charu.
…On 8 August 2018 at 14:49, Cyrille Rossant ***@***.***> wrote:
What we'll need to define clearly is the algorithm specifying (1) when the
zygosities are set and (2) how they are set, depending on the line, the
sequences, the genotype tests, the parents genotypes, including when the
parents are from different lines. It seems this algorithm is not clearly
defined right now and the code is behaving incorrectly in some cases.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#430 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AbP-XDkGyQSBAQDN4gCOFtWoWLe1HtKeks5uOuyAgaJpZM4Vz5ty>
.
--
Charu Reddy
Laboratory Manager
Cortical Processing Laboratory
University College London
The Cruciform Building
Gower Street
London WC1E 6BT
Phone: 020 7679 6147
https://www.ucl.ac.uk/cortexlab
--
|
|
Hi Cyrille, I just spoke with Charu about this. The reason for the unexpected behavior was that some new lines had not had their zygosities rules set up in zygosities.py yet. I don't think there is any problem with our current approach in terms of algorithm definitions, but we should make zygosities.py into a table so that charu/others can easily see it and update it. I think this is simple, and we should do it asap.
Every time any mouse gets a new sequence result, this search is run for that mouse's line and sequences, and zygosities are updated. I believe this is exactly the current logic and as mentioned, the current problems Charu has noted are not related to that logic, but instead related to the fact that the zygosity rules were invisible to her. Is it easy to set this up on alyxdev? If you think we can't get it up today or tomorrow then I'll make a PR with changes to the existing zygosities.py, but I think there's no reason to put this off.... |
|
thanks! I'll try to do that this afternoon |
|
should be done on alyx-dev, see please double-check that it works correctly... |
|
Looks awesome! The interface for adding them is great and the table fields all look correct. I made a test rule and gave a test mouse a new sequence that fit the test, and it worked. Any other parts of the mechanism that you're thinking need testing? I'll go over it briefly with Charu tomorrow too and we'll get back to you. |
|
Whoops, Charu and I just tested this and it seemed not to be working this morning. We tried adding a new rule and then a new sequence for a test mouse, but the mouse's zygosity did not update. We also tried changing a rule and zygosities did not update for existing mice. What determine when the rules are applied? And how are you determining precedence of inferring zygosity from parents versus this table of rules? |
Did that work yesterday?
Indeed, this has not been implemented, should it?
|
|
I thought it did work yesterday. But it's not working now, I just tried again. So e.g. I added a rule for line Ai32cg.Pv and allele Ai32-ChR2, and then gave a genotype test for the appropriate sequence to a mouse. But the zygosity did not update. According to the rule, the mouse below should have gotten Ai32-ChR2 -/-, but as you can see, did not. This is after clicking "save and continue editing" on that subject, and I provide the subject link so you can check it out. In this case I made the rule before adding the test result so I think it should have worked already. But I think if a rule is changed/updated/created then yes, it should update any mice to which the new rule applies. We at least need that for now as some rules should be back-applied to existing mice whose genotype test results are already in the database, and I can also imagine in the future that someone may make a mistake with the rule and want to fix it - after fixing it, they would expect that the new rule would be in force. The question is just whether there's any situation where you'd want to leave mice with the zygosity label they had under the old rule, when you make a new rule - I can't think of a reason you'd want to do that. One slightly tricky aspect may be: if a mouse originally had a determination from the parents about a zygosity, then that zygosity was overwritten by a genotype test rule - but then that zygosity rule is deleted. In that case, it should revert back to the zygosity from the parent. Perhaps the thing to do is: when a rule is updated/created/deleted, then for any mice which match that line and have that genotype test, the zygosity determination should be re-performed from the top: 1) from parents the subsequently 2) from rules. Is that hard to do?
https://alyx-dev.cortexlab.net/admin/subjects/subject/f1e9544a-0aea-4b46-8159-6e6cd50c448e/change/ |
Ah, but the zygosity already existed, so there's a "conflict"? In this case, the default behavior is to skip the rule, but I guess we should change that to enforce the new zygosity. What if you try with a zygosity that does not exist when you apply the rule? I'll implement the other enhancements. |
|
okay I pushed some changes on alyx-dev, could you try again? |
|
K one observation, I see this now in the subject view: "Ai32-ChR2 +/- for
Bovet; Pv-Cre +/- for Bovet" whereas before it was just " Ai32-ChR2 +/- ;
Pv-Cre +/- ". The previous one was correct, there's no need for the subject
name there.
…On Fri, Aug 10, 2018 at 2:02 PM Cyrille Rossant ***@***.***> wrote:
okay I pushed some changes on alyx-dev, could you try again?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#430 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHPUP_hyy5tJEZA-LSernY62Iy-a1DrXks5uPYRwgaJpZM4Vz5ty>
.
|
|
sure, it was helpful for debugging but I'll remove it |
|
I see. Ok, adding a new rule works, and changing the rule reflects properly. Bovet's zygosity is still not updating according to the rule, though...? I even tried deleting the genotype test and re-adding it. Still shows Ai32-ChR2 allele as Het when it should be Absent, according to the new rule. |
|
Ah, it was a bug with zygosity=0 (absent)... try again? |
|
Ok. That's working. I'm now trying to figure out this mouse: If I add Cre test, then given the new rule the PVCre zygosity is getting updated correctly. But, when I remove the Cre test, it goes back to what it was originally, which is PVCre -/-. First thing is that I'm not sure why it had that value originally, since it had that value before I defined any rule about Cre for that line. Second is I'm not sure why it went back to that value. It's not getting the value from the parents (or if it is, then it's a bug) because the PV parent was PV-Cre +/- (so there would be no conclusion about the offspring for this allele. Can you figure out why that mouse has PV-Cre -/-? |
|
hmm I'm not able to reproduce this... :/ I'm going to push an update that makes it easier to navigate individual zygosity and genotype test instances. Maybe you could delete all objects related to this and try again. This is what I did locally and the bug didn't seem to appear. |
|
done, see you have some filters, and you can search by subject |
|
I ran out of time and didn't get to test this more... @charureddy if you are able to test more on alyx-dev this weekend then maybe there's a possibility of a monday morning update? Otherwise we'll have to wait to test more next week, I'm gone this weekend. |
|
Sorry, I am away this weekend too, so I can only do it on Monday. Thanks
Cyrille and Nick!
Charu.
…On 10 August 2018 at 18:52, Nick Steinmetz ***@***.***> wrote:
I ran out of time and didn't get to test this more... @charureddy
<https://github.com/charureddy> if you are able to test more on alyx-dev
this weekend then maybe there's a possibility of a monday morning update?
Otherwise we'll have to wait to test more next week, I'm gone this weekend.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#430 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AbP-XK8lTaL9abxMPKBugazqHpaHf8cEks5uPchagaJpZM4Vz5ty>
.
--
Charu Reddy
Laboratory Manager
Cortical Processing Laboratory
University College London
The Cruciform Building
Gower Street
London WC1E 6BT
Phone: 020 7679 6147
https://www.ucl.ac.uk/cortexlab
--
|
|
what's the status of this? |
Rules are currently hard-coded ; they should be part of the database definition instead. We may need to revisit the schemas.
This is related to a bug reported by @charureddy, the correction of which might be the occasion to rethink the way it's currently implemented.
cc @kdharris101 @nsteinme
The text was updated successfully, but these errors were encountered: