Gh 533/bugfix/pre and post stimulus target voltage #534
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
There are now two vm_delta_mv's, pre_vm_delta_mv and post_vm_delta_mv; mean_first_stability_epoch and mean_last_stability_epoch are now both compared to autobias_v and current_clamp_sweep_qc_features returns pre_vm_delta_mv and post_vm_delta_mv instead of just vm_delta_mv. qc_current_clamp_sweep now has two possible fail tags pertaining to vm_delta, pre_vm_delta_mv and post_vm_delta_mv. measur_vm_delta now takes mean_baseline and target_v as arguments and returns the difference.
qc_current_clamp_sweep now returns unique fail_tags for pre and post Vm delta. Also fixed a small existing bug in qc_current_clamp_sweep that was caught by test_qc_current_clamp_sweep, where load_default_qc_criteria was being called from qc_features.py
Test file sets up two Sweep objects, both with autobias_v of -60, but one with voltage trace pre and post stimulus baselines that are within 1mV of autovias_v and one with voltage trace pre and post stimulus baselines that are greater than 1mV of the autobias_v, passing_v and failing_v respectively. test_sweep_autobias_v tests that the new autobias attribute of the Sweep object is being set, test_measure_vm_delta tests that measure_vm_delta works with autobias_v, test_current_clamp_qc_features tests that current_clamp_sweep_qc_features returns new keys with correct values, and test_qc_current_clamp_sweep test that qc_current_clamp_sweep returns the proper fail_tags.
… 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.
autobias_v was out of order and disrupting the other positional arguments. It has been moved and set to None as default.
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.
|
The three remaining tests are failing tes_mies_nwb_pipeline_output because some sweeps that were passing in the test experiment files are now failing and some that were failing are now passing so there are now differences in the obtained pipeline_output and the expected pipeline_output, but this is an expected consequence of this pull request. |
The change to using autobias_v to determine pre and post stimulus baseline stability has caused some sweeps that were initially failing in the test experiments to now pass and vice versa, so that the obtained json output and the expected json output no longer match. The pipeline_output jsons have now been updated so that the tests in test_mies_nwb_pipeline_output.py will now pass.
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.
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.
…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.
…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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thank you for contributing to the IPFX, your work and time will help to
advance open science!
Overview:
Please let me know if I need to target a different branch. The contribution guide says to target the latest release branch which has the format rc/x.y.z but I did not find a branch with that naming format.
IPFX currently compares the difference between the mean pre and post stimulus baselines to the vm_delta_mv_max qc criteria to determine if the baseline stability passes or fails qc; however, all current clamp sweeps have an autobias target voltage, and it is the differences between the target voltage and each of the mean pre and post stimulus baselines independently that should be compared to the vm_delta_mv_max qc criteria to determine the pass/fail status of each of the pre and post stimulus stabilities. The problem with how IPFX currently QC's the baseline stability is that the mean pre and post stimulus baselines can be more than 1 mV apart from each other but they can each be within 1 mV of the autobias target voltage. The data acquisition software, MIES, monitors the pre stimulus baseline and will only proceed to the stimulus if the pre stimulus baseline is within 1 mV of the target voltage, and it also monitors the post stimulus baseline and stops the sweep once the post stimulus baseline has returned to within 1 mV of the baseline, but occasionally these sweeps that passed in MIES are failed in IPFX, because even though they are each within 1 mV of the baseline, they are sometimes more than 1 mV apart from eachother (and example of this is given in GH issue # 533). This pull requests adds autobias_v as an attribute to the the Sweep object and compares the differences between the autobias_v and each of the mean pre and post stimulus baselines to the vm_delta_mv_max qc_criteria during sweep QC and adds new fail tags accordingly.
Addresses:
Addresses issue #533 #533
Type of Fix:
functionality to not work as expected)
#Note: The issue was opened as a bug, but may better be classified as new feature.
Solution:
This solution introduces autobias_v to the sweep_record in get_sweep_metadata of the MIESNWBData class and uses the get_value() function of the LabNotebookReader to retrieve it. autobias_v is also added as an attribute to the Sweep class so that qc functions can use this on a sweep by sweep basis for checking mean pre and post stimulus baselines against. measure_vm_delta has been modified to accept a generic mean_baseline and target_v and returns the absolute difference. current_clamp_sweep_qc_features has been modified to return new keys for pre_vm_delta_mv and post_vm_delta_mv, and qc_feature_evaluator has been modified to check both pre_vm_delta_mv and post_vm_delta_mv and return unique fail_tags for each when either fails qc.
Changes:
Validation:
I have included test file test_pre_and_post_vm_delta.py, which tests that autobias_v is being set and tests all qc functions that were modified. I also reran the pipeline with my current fix in place on cell Htr3a-Cre_NO152;Ai14-589339.07.09.03, which is the cell referenced in issue #533 that was failing shorts_quare feature extraction because sweep 49 was failing with fail_tag "Vm delta: 1.548 above threshold: 1.000" because the pre and post baseline averages were 1.548 mV apart (-67.205 and -68.752), but each were only about 0.8 mV from the target autobias voltage of -68 mV, and the sweep passed and I was able to get the short_square features without causing any issues or breaking any existing functionality.
Screenshots:
Unit Tests:
Script to reproduce error and fix:
Configuration details:
Checklist
Allen Institute Contribution Guidelines
Numpy Standards
appropriate
Notes:
Please let me know if I need to target a different branch. The contribution guide says to target the latest release branch which has the format rc/x.y.z but I did not find a branch with that naming format.