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

Pre- and Post-stimulus baselines should be compared to target voltage #533

Open
ru57y34nn opened this issue Sep 3, 2021 · 3 comments
Open

Comments

@ru57y34nn
Copy link
Contributor

ru57y34nn commented Sep 3, 2021

Describe the bug
IPFX is not currently considering the taget autobias command voltage when QCing the pre- and post-stimulus baselines, and this is occasionally causing IPFX to fail sweeps that passed in MIES. In certain cases, this has prevented the extraction of certain sets of features.

Currently, IPFX measures the mean pre- and post-stimulus baselines and then gets the absolute difference between them (vm_delta_mv). Any sweeps that have a vm_delta_mv greater than 1 mV are failed during QC. Whereas, MIES first gets the target autobias command voltage for the given sweep and then measures the mean pre- and post-stimulus baselines and gets the absolute difference between the target autobias command voltage and the pre- and post-stimulus baselines independently. This way, MIES is checking that the pre- and post-stimulus baselines are each within 1 mV of the target voltage. The problem with how IPFX is doing this is that the pre- and post-stimulus baselines can each be within 1 mV of the target voltage, while the difference between the pre-and post-stimulus baselines can be greater than 1mV.

Example experiment demonstrating the issue:
Cell name: Htr3a-Cre_NO152;Ai14-589339.07.09.03
Path to NWB file: \allen\programs\celltypes\production\mousecelltypes\prod3258\Ephys_Roi_Result_1121464496\
(See attached image: Htr3-Cre_NO152;Ai14-589339.07.09.03_baseline_issue)

Htr3-Cre_NO152;Ai14-589339 07 09 03_baseline_issue

Sweep 49, the rheobase sweep (hero sweep) had a target autobias command voltage of -68 mV and the mean pre-stimulus baseline of -67.205 and a mean post-stimulus baseline of -68.752, so both baselines were within 1mV of the target voltage, but the delta between them was 1.548 mV, so this sweep was failed during QC and as a result, the shortsquare features were not extracted.

To Reproduce
I have attached an image from a notebook (see attached image: vm_delta_mv_notebook) that I am using to demonstrate the current behavior for how vm_delta_mv is currently determined (circled in red), and how that should be replaced with pre_vm_delta_mv and post_vm_delta_mv (circled in green).

vm_delta_mv_notebook

Expected behavior
Since we have a target voltage set, it is important that we ensure that the pre- and post-stimulus baselines are independently within +/- 1 mV of the target autobias voltage. The important metric here is not that they are within 1 mV of eachother, but that they are each within 1 mV of the target voltage.

Actual Behavior
No errors are produced by this bug, it is just incorrectly doing a comparison between the pre- and post-stimulus baselines and returning a single vm_delta_mv value when it should be comparing each of those independently to the target autobias voltage and returning pre_vm_delta_mv and post_vm_delta_mv values.

Environment (please complete the following information):

  • OS & version: Windows 10
  • Python version 3.7.10
  • AllenSDK version 2.10.2

Additional context
I think that the target autobias command voltage should be added to the sweep_record dictionary of the get_sweep_metadata() function in the MIESNWBData class in mise_nwb_data.py with:
“autobias_v” = self.notebook.get_value(“Autobias Vcom”, sweep_number, None)

so that the sweep() function in ephys_data_set.py can get it via:
sweep_metadata = self._data.get_sweep_metadata(sweep_number)

and then set it as:
autobias_v = sweep_metadata[“autobias_v”] (The Sweep class in sweep.py will also need to add autobias_v as an attribute)

The measure_vm_delta() function in qc_features will need to take in a baseline and the target voltage and return the difference between those, rather than taking the pre- and post-stimulus baselines and returning the difference between the two baselines, and the current_clamp_sweep_qc_features() function in qc_feature_extractor.py will need to return “pre_vm_delta_mv” and “post_vm_delta_mv” in the qc_features dictionary, rather than just “vm_delta_mv”.

Do you want to work on this issue?
I am working on a pull request for this now which should at least give a good idea of what we are looking for.

@ru57y34nn ru57y34nn added the bug label Sep 3, 2021
@tmchartrand
Copy link
Collaborator

@ru57y34nn this may be a reasonable change to make, but I certainly wouldn't call it a bug. This use of delta between pre and post is consistent with our published QC steps in the ephys whitepaper and the first mouse IVSCC publication, so it's really MIES that has departed from the standard here.
I'm certainly open to this as a reasonable slight relaxation of that criteria (we're already relaxing the delta mv cutoff for processing human data, and I believe Nathan has in some cases too). But we should probably be clear that this is a desired direction and have signoff from at least Tim and Nathan before putting effort in on this.

@ru57y34nn
Copy link
Contributor Author

Thanks @tmchartrand for the additional information. I am wondering then if I should resubmit this but as a feature request rather than a bug, and if we should keep vm_delta_mv, and just add pre_vm_delta_mv and post_vm_delta_mv. It seem to me that there should at least be a check somewhere to make sure that the pre- and post-stimulus baselines are within range of the target voltage. Perhaps we could also just relax the qc criteria for vm_delta_mv to +/- 2 mV, since the pre- and post-stimulus baselines can each be up to 1 mV from the target and so no more than 2 mV from eachother?

@timjarsky
Copy link

@tmchartrand baseline potentials are compared to the target voltage to ensure the cell's RMP hasn't changed from the break-in potential (the potential that's closest to the cell's physiological RMP). So from this point of view, I think it makes sense for the pre-pulse baseline and post-pulse baseline to be evaluated against the same criteria.

ru57y34nn added a commit to ru57y34nn/ipfx that referenced this issue Sep 21, 2021
Getting Autobias Vcom from notebook in get_sweep_metadata in MIESNWBData
class and added to sweep function of EphysDataSet class as autobias_v and
added as an attribute of Sweep class in sweep.py.
ru57y34nn added a commit to ru57y34nn/ipfx that referenced this issue Sep 23, 2021
… is None

Check both mean_baseline and target_v and return None if either is None.
Add new test test_none_autobias_v to test handling of None autobias_v,
and fixed bug in test script when setting up test Sweeps.
ru57y34nn added a commit to ru57y34nn/ipfx that referenced this issue Sep 23, 2021
autobias_v was out of order and disrupting the other positional arguments.
It has been moved and set to None as default.
ru57y34nn added a commit to ru57y34nn/ipfx that referenced this issue Sep 23, 2021
First check that autobias_v is present in sweep_metadata.keys() before assigning value in
Sweep(), set to None if not present. Also added autobias_v as None in
test_get_sweep_metadata from test_mies_nwb_data.py so that expected
result and obtained result match.
ru57y34nn added a commit to ru57y34nn/ipfx that referenced this issue Oct 8, 2021
If autobias_v is None for a given sweep, then default to previous method
of measuring delta between pre and post-stimulus baselines as vm_delta_mv.
ru57y34nn added a commit to ru57y34nn/ipfx that referenced this issue Oct 8, 2021
Setup a test sweep with autobias_v = None and added assert statements to
functions to verify outputs for sweeps with autobias_v = None. Removed
test_none_autobias_v function as no longer returning delta = None if
autobias_v = None, but returning delta between mean pre and post-
stimulus baselines.
ru57y34nn added a commit to ru57y34nn/ipfx that referenced this issue Oct 8, 2021
…b_pipeline_output

Pipeline_output jsons have changed again since adding back in the
previous method of measuring vm delta between mean pre and post-stimulus
baselines and defaulting to this to handle sweeps where autobias_v = None.
ru57y34nn added a commit to ru57y34nn/ipfx that referenced this issue Oct 15, 2021
…mv fail tags

Consolidate pre_vm_delta_mv and post_vm_delta_mv into single
vm_autobias_delta_mv metric by taking max value of the two. Also remove
pre_vm_delta_mv and post_vm_delta_mv fail tags and assign same
vm_delta_mv fail tag to all delta_mv fail metrics regardless of autobias.
This change makes it so that there will not need to be any LIMS changes
by not adding any additional fail tags. Also revert measure_vm_delta()
back to original state and generalized argument names and update
test_pre_and_post_vm_delta.py with checks for ramp sweeps and update
pipeline_output jsons.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants