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

Fix missing electrode 0 when in bath sweep was ran with smoketest sti… #510

Conversation

ru57y34nn
Copy link
Contributor

@ru57y34nn ru57y34nn commented Mar 26, 2021

…mulus

This commit extends the 'extract_electrode_0' function and tries to
determine if the 'in-bath' sweep was ran with the 'EXTPSMOKET' stimulus
when the 'EXTPINBATH' stimulus is not found.

If 'EXTPINBATH' is not found then measure the seal of all 'EXTPSMOKET'
sweeps that occur before the first instance of 'EXTPCllATT' and only if
the resistance of the seal is below 10 MOhms then call
'measure_electrode_0'. Electrode 0 is not available if no 'EXTPSMOKET'
sweeps occurred before the first 'EXTPCllATT' sweep, the resistance is
above 10 MOhms, or if no 'EXTPSMOKET' or no 'EXTPCllATT' sweeps found.

Thank you for contributing to the IPFX, your work and time will help to
advance open science!

Overview:

Many of the recent IVSCC pipeline experiments are failing cell QC with fail tag
"electrode_0_pa missing value". This issue occurred because a software issue,
which has since been fixed, that was causing the in-bath sweep to be collected
with the 'EXTPSMOKET' stimulus instead of the 'EXTPINBATH' stimulus. In fact,
these stimuli are the same wave, but with different names so that the stage
of the experiment can be determined simply by the stimulus wave name.
Most of these failing experiments actually contain the in bath sweep, but it
was ran with the 'EXTPSMOKET' stimulus. IPFX needs to be able to look for
in-bath sweeps collected with the 'EXTPSMOKET' stimulus when the
'EXTPINBATH' stimulus is not found.

Addresses:

Addresses issue #509

Type of Fix:

  • New feature (non-breaking change which adds functionality)

Solution:

When 'extract_electrode_0' does not find any 'EXTPINBATH' stimulus sweeps,
then it will look through any 'EXTPSMOKET' stimulus sweeps that occurr
before the first instance of the 'EXTPCllATT' sweep and check the resistance.
If the resistance is low, < 10 MOhms, then the pipette is determined to
have been in the bath and the 'measure_electrode_0' is called and the value
is returned. Electrode 0 is determined to be unavailable if no 'EXTPSMOKET'
sweeps occurred before the first 'EXTPCllATT' sweep, if no 'EXTPSMOKET'
sweeps have a resistance < 10 MOhms, or if no 'EXTPSMOKET', or no
'EXTPCllATT' sweeps are present.

Changes:

  • Added self.smoketest_names to StimulusOntology to find 'EXTPSMOKET'
  • Extended extract_electrode_0 function to try to get electrode 0 from
    'EXTPSMOKET' stimulus sweeps when 'EXTPINBATH' is not found

Validation:

I have tested this on the 600+ pipeline experiments that are affected by
this issue and have validated that it is only able to extract the electrode 0
from 'EXTPSMOKET' sweeps when the 'EXTPINBATH' is not found and
if the resistance is <10 MOhms (the range of an open pipette in the bath).
I wrote some unit test that use experiments where the 'EXTPINBATH' is
missing and either:

  • there is a 'EXTPSMOKET' sweep but the resistance is too high
  • there are multiple 'EXTPSMOKET' sweeps, some occurring after
    'EXTPCllATT', but only one before the 'EXTPCllATT' sweep that has
    a resistance < 10 MOhms
  • there are multiple 'EXTPSMOKET' sweeps, all occurring before the
    'EXTPCllATT' sweep but only one with resistance < 10 MOhms

Unit Tests:

A note about my unit tests: They are not hermetic as I needed data_sets
to test the extract_electrode_0 function so I am using nwb files from LIMS.
Please let me know if this is not acceptable and if there is an alternative
solution I could use.

Notes:

Please let me know if there are any issues with my solution so that we
can address those ASAP. A lot of IVSCC pipeline data is being affected
by this issue and we are in need of a fix that will accomplish the
functionality that I have added here. Thanks

…mulus

This commit extends the 'extract_electrode_0' function and tries to
determine if the 'in-bath' sweep was ran with the 'EXTPSMOKET' stimulus
when the 'EXTPINBATH' stimulus is not found.

If 'EXTPINBATH' is not found then measure the seal of all 'EXTPSMOKET'
sweeps that occur before the first instance of 'EXTPCllATT' and only if
the resistance of the seal is below 10 MOhms then call
'measure_electrode_0'. Electrode 0 is not available if no 'EXTPSMOKET'
sweeps occurred before the first 'EXTPCllATT' sweep, the resistance is
above 10 MOhms, or if no 'EXTPSMOKET' or no 'EXTPCllATT' sweeps found.
@MatthewAitken
Copy link
Member

Looks good, but needs two changes:

  • Target dev instead of master (I'll create a new release for this functionality so it can be put into production)
  • test data in LIMS is indeed bad. Please create new directory "ipfx/tests/data/" and place the nwb files there. Before committing the test data please remove as much data from the nwb files as possible (ideally keep the files under 10MB each)

When it's done I'll handle all the release work! Thanks!

This commit refactors the code that was added to extract_electrode_0 for
finding inbath sweeps ran with the smoketest stimulus. The previous unit
tests were not hermetic so this refactor and the couple of new functions
allow for hermetic testing.

new functions: filter_smoketests filters out any smoketest sweeps that
occurred after the cell attached sweep and minimum_e0 finds the minimum
absolute value from a list of e0 values; these e0 values are measured
from the smoketest sweeps that occurred after the first cell attached
sweep if the resistance of the pipette is less than 10 MOhms.
These new tests are hermetic and contain all of their own mock data
and test the filter_smoketests and minimum_e0 functions in
qc_feature_extractor.py.
@ru57y34nn ru57y34nn changed the base branch from master to dev April 8, 2021 17:45
MatthewAitken
MatthewAitken previously approved these changes Apr 8, 2021
Copy link
Member

@MatthewAitken MatthewAitken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I asked Sergey to also peak at it, and then I'll merge it in and create a new IPFX release!

@sgratiy
Copy link
Contributor

sgratiy commented Apr 8, 2021

@ru57y34nn Since this is an issue with the data I can see an alternative approach to correct the annotation of the nwb files and LIMS records? Would not that be a better approach?

Copy link
Contributor

@sgratiy sgratiy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel that since we are dealing with the data issue, we should fix data, not the code processing it.
But if that is not feasible, then we must at least document the change to the QC evaluation. IPFX does not have this detailed documentation because we rely on the published white paper Electrophysiology Overview. But if we start to diverge from it we must describe what we do. At least we should describe how we deviate from the main path.

bath_data = data_set.sweep(sweep)
seal = qcf.measure_seal(bath_data.v, bath_data.i, bath_data.t)
print(seal)
if abs(seal) < 0.01:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you determine this threshold? Can it be set in terms of the existing qc parameters? What would happen if we did not include this condition here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to this point, we did not have ad-hoc thresholds in the code and I would prefer us keeping it that way. Thus if you can think of alternative to setting a threshold 0.01 that would be preferable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgratiy I will look into this and see if there is a way that we can correct the data rather than modifying IPFX. The threshold is there because the EXTPSMOKET sweep is usually run with the pipette out of the bath, but for a period of time it was also being run once the pipette was in the bath in place of the EXTPINBATH. We need a way to determine if the pipette was in the bath during the EXTPSMOKET before using it to measure the electrode_0 value, and a low resistance seal (10 MOhm, or 0.01 Ohm) is the only way I can see to do that. The resistance of an open pipette tip in the bath is usually around 3-8 MOhms and will never exceed 10 MOhms (you simply can't patch with a pipette that has a resistance higher than 10 MOhms), and calculating a seal from a smoke test where the pipette is out of the bath will result in a very high seal (usually several GOhms). This is the best way I could figure out how to find EXTPSMOKET sweeps where the pipette was in the bath rather than out of the bath. I will give an update here once I figure out if there is an alternative route we can go here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgratiy I see that the white paper states that "Thick walled borosilicate glass (...) electrodes were manufactured (...) with a resistance of 3 to 7 MOhms". Would it be acceptable if I made this threshold a constant and made the comparison to that (instead of the arbitrary 0.01 Ohm threshold defined in the middle of the code)?

Also, while I see where you are coming from about fixing the data rather than changing the code that processes it, I can see that the extract_electrode_0 function isn't entirely robust as it is, in that it doesn't check the resistance of the pipette to ensure that it is in the range mentioned in the white paper. I think that the resistance check I am doing needs to also be applied even when getting the sweep with the proper in-bath name, EXTPINBATH. I could add another function to check pipette resistance to qc_features.py and this could be called in extract_electrode_0 to ensure that the resistance of the pipette is in range before getting the baseline current (e0). As it is right now, any sweep that is ran with the EXTPINBATH stimulus will have the baseline current extracted even if the pipette was not in the bath, or in some other configuration, and if that baseline current is within range then electrode_0 will pass QC.

The Ephys team's thoughts on this are that the data are immutable and that the purpose of ipfx is to make use of the data and that it should be able to handle certain errors in the data. I think that it is reasonable for ipfx to check through the smoketest sweeps, EXTPSMOKET, when no sweep with the in-bath name, EXTPINBATH, is found, and see if one of those sweeps was collected with the pipette in the bath, especially because these stimulus waves are exactly the same. The EXTPSMOKET and the EXTPINBATH stimulus waves are exactly the same, just named differently to more easily identify where in the experiment the user should be; however it should not be assumed the user always runs the appropriate stimulus.

I don't think that what I am proposing here deviates from the white paper at all but let me know what you think Sergey, thanks!

@MatthewAitken MatthewAitken dismissed their stale review April 8, 2021 23:58

sergey's point

perform resistance check on EXTPINBATH and EXTPSMOKET sweeps depending
on which is used for electrode 0.
This commit address the issue of the 0.01 threshold for pipette resistance.
This threshold has been moved to a constant called max_pipette_resistance
and is now in units of MOhms and is set to 7.5. Actual pipette resistance
is now found with measure_input_resistance from qc_features.py, which
returns the resistance in units of MOhms.
@sgratiy
Copy link
Contributor

sgratiy commented May 15, 2021

Closing this PR without merging. Will correct data instead to solve the underlying issue.

@sgratiy sgratiy closed this May 15, 2021
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

Successfully merging this pull request may close these issues.

3 participants