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 impedance #402

Merged
merged 6 commits into from
Jul 11, 2024
Merged

Conversation

AurelienJaquier
Copy link
Collaborator

@AurelienJaquier AurelienJaquier commented Jul 11, 2024

Description

  • When stimulus starts at 0 for impedance (which can happen often), use steady_state_voltage_stimend to get holding voltage, and equivalent for holding current. Since at the end of the stimulus, we expect the current and voltage to vary rapidly around the holding value, this is a good enough proxy to get it.
  • implemented test_steady_state_current_stimend, along with documentation and test
  • fixed current interpolation
  • fixed stim_end value in impedance test data file

Checklist:

  • Unit tests are added to cover the changes (skip if not applicable).
  • The changes are mentioned in the documentation (skip if not applicable).
  • CHANGELOG file is updated (skip if not applicable).

Jaquier Aurélien Tristan added 2 commits July 11, 2024 14:49
@AurelienJaquier AurelienJaquier self-assigned this Jul 11, 2024
Jaquier Aurélien Tristan added 2 commits July 11, 2024 15:03
@codecov-commenter
Copy link

codecov-commenter commented Jul 11, 2024

Codecov Report

Attention: Patch coverage is 88.63636% with 5 lines in your changes missing coverage. Please review.

Project coverage is 92.24%. Comparing base (2cddd26) to head (9ed2241).
Report is 23 commits behind head on master.

Files Patch % Lines
efel/cppcore/Subthreshold.cpp 86.95% 0 Missing and 3 partials ⚠️
efel/pyfeatures/pyfeatures.py 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #402      +/-   ##
==========================================
+ Coverage   91.73%   92.24%   +0.51%     
==========================================
  Files          36       39       +3     
  Lines        6278     7389    +1111     
  Branches     2033     2293     +260     
==========================================
+ Hits         5759     6816    +1057     
- Misses        266      289      +23     
- Partials      253      284      +31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AurelienJaquier AurelienJaquier requested a review from ilkilic July 11, 2024 13:55
CHANGELOG.rst Outdated
5.7.1 - 2024-07
---------------

- When stimulus starts at 0 for impedance (which can happen often), use test_steady_state_voltage_stimend to get holding voltage, and equivalent for holding current.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you meant steady_state_voltage_stimend

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done in latest commit

size_t start_index = distance(times.begin(), start_it);
size_t stop_index = distance(times.begin(), stop_it);

double mean = accumulate(currents.begin() + start_index,
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can even compute the mean inside the if block

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done in latest commit

Copy link
Collaborator

Choose a reason for hiding this comment

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

Formidable! Thank you

Copy link
Collaborator

@ilkilic ilkilic 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!

@AurelienJaquier AurelienJaquier merged commit 9f9ad41 into BlueBrain:master Jul 11, 2024
20 checks passed
@AurelienJaquier AurelienJaquier deleted the fix-impedance branch July 11, 2024 15:14
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