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

Change labman/labman/db/process.py generate_echo_picklist() call to _format_picklist() to pass in 20000 (nL) as max_vol_per_well #438

Closed
charles-cowart opened this issue Apr 5, 2019 · 7 comments · Fixed by #515

Comments

@charles-cowart
Copy link
Collaborator

From RodolfoSalido:

I quantified my Shotgun libraries and generated a pooling file using Lab Control. I only noticed one discrepancy between the pooling file generated by Lab Control and that generated by our Jupyter notebook. Our Jupyter notebook is designed to pool into a destination well until it reaches a certain volume (30µL) and then it cycles to the next well. The Lab Control pooling function seems to have a value of 60µL for the max_vol_per_well parameter instead of 30µL.

@charles-cowart charles-cowart self-assigned this Apr 5, 2019
@charles-cowart
Copy link
Collaborator Author

From RodolfoSalido's output:

283 rows do not match between the two outputs, out of the total of 384. Of the rows that don't match, they appear to differ by 1/100th of a unit in transfer volume, and the destination wells differ. Awaiting confirmation from Rodolfo.

File A:
1,384LDV_AQ_B2_HT,B2,,262.69,NormalizedDNA,A1
1,384LDV_AQ_B2_HT,B8,,363.07,NormalizedDNA,A1

File B:
1,384LDV_AQ_B2_HT,B2,,262.68,NormalizedDNA,A1
1,384LDV_AQ_B2_HT,B8,,363.06,NormalizedDNA,A1

@charles-cowart
Copy link
Collaborator Author

Confirmed - here are the transfer volumes summed for each of the destination wells for each file:
From Rodolfo:

The real granularity of our liquid handler is 2.5 nL increments do 1/100th unit discrepancies are not a problem. If you sum all the volumes that go into any specific destination well, the addition shouldn’t exceed 30 micro-liters.

Confirmed by summing the transfer volumes in each file across their destination wells:

FIle 1:
A1 = 29825.000000
A2 = 29912.560000
A3 = 29770.010000
A4 = 24522.410000

File 2:
A1 = 59737.560000
A2 = 54292.360000

@charles-cowart
Copy link
Collaborator Author

charles-cowart commented Apr 6, 2019

It looks like the parameters set up in 'HTML_POOL_PARAMS_SHOTGUN' set 'blank-vol-' to empty string for the 'equal' pooling function. This in turn sets 'PoolingProcess.compute_pooling_values_eqvol().total_vol' to 60ul by default.

Perhaps changing the maximum total volume of the destination well to 30ul is a matter of setting the 'Pool all blanks at volume' property. Testing on maptest.

image

@charles-cowart
Copy link
Collaborator Author

Issues w/maptest when leaving this page and coming back to it. The robustness of this page needs to be addressed (See #361)

@charles-cowart charles-cowart added this to the Full Launch milestone Apr 6, 2019
@charles-cowart
Copy link
Collaborator Author

Just to reiterate, I believe #361 should be resolved first, as it's not easy to get back results to confirm the correctness of a bug fix with the known beta issue.

@AmandaBirmingham
Copy link
Collaborator

TL;DR: change

https://github.com/jdereus/labman/blob/a6777cc8b29946c06599b5d3e78a5aeaf4541427/labman/db/process.py#L2559

to

return PoolingProcess._format_picklist(vol_sample, max_vol_per_well)

and this problem should be fixed. Explanation below.


I believe Rodolfo pointed us on the right road here with his reference to the variable name max_vol_per_well. I see PoolingProcess has this function:

https://github.com/jdereus/labman/blob/a6777cc8b29946c06599b5d3e78a5aeaf4541427/labman/db/process.py#L2497

Note the default, which is in nL:

https://github.com/jdereus/labman/blob/a6777cc8b29946c06599b5d3e78a5aeaf4541427/labman/db/process.py#L2505-L2506

60000 nL = 60 microliters (μL) :)

This method is called by generate_echo_picklist:

https://github.com/jdereus/labman/blob/a6777cc8b29946c06599b5d3e78a5aeaf4541427/labman/db/process.py#L2542-L2548

Here, the default is 30000 nl or 30 μL, which is the value the lab actually wants to use, so you'd think we'd be golden, except ... the max_vol_per_well value IS NEVER USED. Instead, _format_picklist is called without an explicit max_vol_per_well value:

https://github.com/jdereus/labman/blob/a6777cc8b29946c06599b5d3e78a5aeaf4541427/labman/db/process.py#L2559

Therefore the 60 μL default in _format_picklist takes over, and Bob's your uncle ...

@AmandaBirmingham
Copy link
Collaborator

From: Rodolfo Salido
Date: Monday, April 15, 2019 at 11:18 AM
To: "Birmingham, Amanda"
Cc: Charles Cowart , Daniel McDonald
Subject: Re: Shotgun Libraries pooling.

Hello Amanda,
We don’t need this parameter to be editable, we can have it hardcoded. Karenina has been using 20µL as her default and I’ve been pooling 30µL. We changed it from 60µL to a lower 20-30 µL because we changed destination plates and the newer plates hold less volume.

We don’t anticipate any future changes to our pooling procedure. After talking with Karenina, I believe that going with the more conservative 20µL max_vol_per_well will be the better choice.

-Rodolfo

On Apr 15, 2019, at 11:09 AM, Birmingham, Amanda wrote:

Hi—
I’ve looked into this, and I am 90% sure I know where the issue is coming from (I’ve put details on #438
). Making the code use max_vol_per_well = 30µL instead of 60µL should be the work of a moment (although updating the tests may take a little longer) ☺.

HOWEVER, I am also 90% sure there is absolutely no way for a user to change this value through the GUI—it is hardcoded. Rodolfo, is max_vol_per_well a setting that the lab ever has changed, or anticipates changing in the future? If so, we probably need to add it to the user interface somewhere and hook up the back end to take that input … Lemme know what you think.
Thanks,
Amanda

@AmandaBirmingham AmandaBirmingham changed the title Shotgun libraries pooling discrepancy Change labman/labman/db/process.py generate_echo_picklist() call to _format_picklist() to pass in 20000 (nL) as max_vol_per_well Apr 15, 2019
AmandaBirmingham added a commit that referenced this issue May 8, 2019
AmandaBirmingham added a commit that referenced this issue May 9, 2019
wasade pushed a commit that referenced this issue May 10, 2019
* Fixes #438 and #505

* Modified test case data per reviewer comment

* Fixed small code review comment from PR #504

* Corrected undesired shadowing of built in "input" variable

* Addresses code-review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants