-
Notifications
You must be signed in to change notification settings - Fork 38
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
update C++ codebase for handling of feature dependencies [vcs: #minor] #334
Merged
Changes from 93 commits
Commits
Show all changes
94 commits
Select commit
Hold shift + click to select a range
0e6dbcd
throw runtime_error, rm exit(1) in featuretype&calcfeatures
anilbey 5cc3c5e
rename type->input_type
anilbey 2a84582
throw EfelAssertionError to avoid exit(-1)
anilbey 84626df
add test to trigger C++->Python AssertionError
anilbey 954613a
remove featurename.find(";") check
anilbey e9cbcc8
typo in docstring
anilbey 443e083
add feature's name to the error message
anilbey f11e36d
Revert "remove featurename.find(";") check"
anilbey 108ca85
remove setversion function
anilbey 71f687d
draft: removing alternative wildcard syntax
anilbey 2ba460d
merge master
anilbey 3748628
remove variable features in LibV5
anilbey 9fa5fa4
remove variable (alias) features in LibV2
anilbey f07895b
simplify feature pointers representation in cppcore, remove wildcards
anilbey 655cd03
make AddUniqueItem void
anilbey 34cfb4d
remove empty function getDependencyList
anilbey b8f629a
directly check stream's state after opening the file
anilbey d91d476
remove dead code in efel and cfeature
anilbey 9172cc7
merge efel into cppcore
anilbey 8ed52a8
add template to getParam
anilbey 786f636
add getFeatures fn to get all dependent features
anilbey 6a80aeb
LibV5.cpp update until time_to_last_spike
anilbey cf52d1d
libv5 update ISI computations
anilbey 1abfbac
LibV5 remove ISI first, second etc. duplication
anilbey 32eb094
calculateInvISI throw except instead of return 0
anilbey 13c21f6
use getFeatures in depolarized_base
anilbey 9ad88fd
use getFeatures in steady_state_hyper
anilbey 4de7876
depolarized_based consider retVal==0 failure
anilbey 3c86f4e
use getFeatures in LibV2
anilbey 4d36018
use getFeatures in LibV1
anilbey 65e8f60
using to replace scope resolution
anilbey d0084a3
define distinct errors: FeatureComputationError, EmptyFeatureError
anilbey 84b3020
use getFeature/s in LibV5 for consistent handling of edge cases
anilbey 841f2b3
remove unnecessary initializer_list
anilbey ffd9d03
min_AHP_indices to use getFeatures
anilbey 90a5dc6
AHP_depth_abs with getFeature
anilbey 749e002
spike_half_width to use getFeatures
anilbey 932b1b1
AP_begin_indices to use getFeatures
anilbey 35a8450
use getFeatures in current_base
anilbey 0510877
getfeatures in burst_begin_indices
anilbey 378c278
decay_time_constant_after_stim to use getFeatures
anilbey 8ded463
libv5 exception handling
anilbey a21b653
remove getVec completely
anilbey f3b3a73
Merge branch 'master' into wildcards
anilbey 4e6514b
make format
anilbey f7425b8
Merge branch 'master' into wildcards
anilbey d1101d9
replace throw EmptyFeatureError with return -1 for consistency
anilbey a4335ec
Merge branch 'wildcards' of https://github.com/BlueBrain/eFEL into wi…
anilbey bde76af
remove redundant else condition
anilbey d6848c3
avoid fetching ignore_first_ISI twice
anilbey 3ca2801
in the spikewidth2 error message mention spikewidth2
anilbey 9b9d382
update maximum_voltage's success case to be same asminimum_voltages
anilbey d5d5ed7
make peakvoltage vector a reference in amp_drop_first_last
anilbey e7907c8
remove obsolete empty feature checks in spike_width1
anilbey e790e16
remove else after throw
anilbey f3adcc3
update new line in code comment for readability
anilbey 83a57b1
replace GErrorStr+1 and return -1 via exception
anilbey 444bfe7
add test to cover single spike case in min_voltage_between_spikes
anilbey 5a5c22d
cover stimulus_current==0.0 edge case in ohmic resistance features
anilbey d7c576f
remove else after throw in 3 LibV5 places
anilbey 944140a
typo in test name
anilbey b3a33f3
test_ohmic_input_resistance_zero_stimulus_current update stim params
anilbey 1253dd3
update fail case for spikecount & spikecount_stimint
anilbey 5784c06
add test calling all features on constant trace
anilbey c101450
remove re imported import in test
anilbey 2ff86ed
merge master into wildcards
anilbey 267220d
impute Spikecount missing val to 0.0 in GranuleCellDeap1.ipynb
anilbey 9e9286f
remove unnecessary variable
anilbey 241a2d7
move Spikecount implementation to Python
anilbey 46007d2
change doc of Spikecount as Python efeature
anilbey 818e3a1
move Spikecount_stimint to Python
anilbey f778c6a
add validation module with check_ais_initiation
anilbey eebc16e
add bpap_attenuation as a multitrace feature
anilbey 31b7212
move burst_number and strict_burst_number to Python
anilbey 4c1aae3
update impedance, remove spikecount is None check
anilbey d287792
add CHANGELOG.md
anilbey 7a2594d
Merge branch 'master' into wildcards
anilbey 133a48e
Merge branch 'master' into wildcards
anilbey cc4f705
docs: add multitrace and validation modules to API
anilbey c02f4c9
Merge branch 'master' into wildcards
anilbey df224bd
Docs: add name change warnings
anilbey c7ff80a
Merge branch 'wildcards' of github.com:BlueBrain/eFEL into wildcards
anilbey 082f205
add int/double template instantiations to cfeature
anilbey c7a4cc9
remove getDistance_cpp, use python implementation
anilbey 5f8881b
move trace_check to pyfeatures
anilbey b6940d1
lint fix
anilbey 87cc375
add from __future__ import annotations
anilbey 41b087e
add stimulus_current to test_allfeatures_on_constant_V
anilbey f39e7eb
allow both spike_count and Spikecount
anilbey 489c247
move validation.py inside pyfeatures
anilbey c6033de
Revert "add stimulus_current to test_allfeatures_on_constant_V"
anilbey 2682494
add spike_count expect it to be 0 for allfeatures_on_constant_voltage
anilbey 43bf8b3
encourage use of spike_count instead of Spikecount
anilbey bc15caa
update deprecation note in the docs
anilbey File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,4 +21,4 @@ bin | |
*.ipynb_checkpoints | ||
lib | ||
fllog.txt | ||
*.DS_Store | ||
*.DS_Store |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
|
||
# Changelog | ||
All notable changes to this project will be documented in this file. | ||
|
||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), | ||
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | ||
|
||
|
||
## [5.4.0] - 2024-01 | ||
|
||
### C++ changes | ||
- New C++ function `getFeatures` replaced `getVec`. | ||
- `getFeatures` automatically handles failures & distinguishes empty results from failures. | ||
- Centralized error handling in `getFeatures` shortens the code by removing repetitions. | ||
- C++ features' access is restricted. Read-only references are marked `const`. | ||
- Removed wildcard features from C++ API. Use of Python is encouraged for that purpose. | ||
|
||
### Python changes | ||
- `bpap_attenuation` feature is added to the Python API. | ||
- `Spikecount`, `Spikecount_stimint`, `burst_number`, `strict_burst_number` and `trace_check` features migrated to Python from C++. | ||
- `check_ais_initiation` is added to the Python API. |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,8 @@ Submodules | |
|
||
api | ||
io | ||
pyfeatures.multitrace | ||
pyfeatures.pyfeatures | ||
pyfeatures.validation | ||
units | ||
settings |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -150,19 +150,21 @@ time from stimulus start to last spike | |
else: | ||
time_to_last_spike = 0 | ||
|
||
`LibV1`_ : Spikecount | ||
~~~~~~~~~~~~~~~~~~~~~ | ||
`Python efeature`_ : spike_count | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
number of spikes in the trace, including outside of stimulus interval | ||
|
||
- **Required features**: LibV1:peak_indices | ||
- **Units**: constant | ||
- **Pseudocode**: :: | ||
|
||
Spikecount = len(peak_indices) | ||
spike_count = len(peak_indices) | ||
|
||
`LibV5`_ : Spikecount_stimint | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
**Note**: In the future this feature will be called "spike_count". | ||
|
||
`Python efeature`_ : spike_count_stimint | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
number of spikes inside the stimulus interval | ||
|
||
|
@@ -171,7 +173,9 @@ number of spikes inside the stimulus interval | |
- **Pseudocode**: :: | ||
|
||
peaktimes_stimint = numpy.where((peak_time >= stim_start) & (peak_time <= stim_end)) | ||
Spikecount_stimint = len(peaktimes_stimint) | ||
spike_count_stimint = len(peaktimes_stimint) | ||
|
||
**Note**: In the future this feature will be called "spike_count_stimint". | ||
|
||
`LibV5`_ : number_initial_spikes | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
@@ -343,23 +347,6 @@ The adaptation index is zero for a constant firing rate and bigger than zero for | |
ISI_sub = ISI_values[1:] - ISI_values[:-1] | ||
adaptation_index = numpy.mean(ISI_sum / ISI_sub) | ||
|
||
|
||
`LibV5`_ : check_AISInitiation | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice to keep this feature. Some users have used it before: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is reimplemented as a Python function. |
||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Check initiation of AP in AIS | ||
|
||
- **Required features**: t, V, stim_start, stim_end, AP_begin_time, AP_begin_time;location_AIS | ||
- **Units**: constant | ||
- **Pseudocode**: :: | ||
|
||
if len(AP_begin_time) != len(AP_begin_time;location_AIS): | ||
return None | ||
for soma_time, ais_time in zip(AP_begin_time, AP_begin_time;location_AIS): | ||
if soma_time < ais_time: | ||
return None | ||
return [1] | ||
|
||
`LibV1`_ : burst_mean_freq | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
|
@@ -419,8 +406,8 @@ The burst detection can be fine-tuned by changing the setting strict_burst_facto | |
) | ||
) | ||
|
||
`LibV1`_ : burst_number | ||
~~~~~~~~~~~~~~~~~~~~~~~ | ||
`Python efeature`_ : burst_number | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
The number of bursts | ||
|
||
|
@@ -430,8 +417,8 @@ The number of bursts | |
|
||
burst_number = len(burst_mean_freq) | ||
|
||
`LibV5`_ : strict_burst_number | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
`Python efeature`_ : strict_burst_number | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
The number of bursts | ||
|
||
|
@@ -1287,98 +1274,6 @@ Slope of the V, dVdt phasespace plot at the beginning of every spike | |
range_min_idxs = AP_begin_indices - AP_phseslope_range | ||
AP_phaseslope = (dvdt[range_max_idxs] - dvdt[range_min_idxs]) / (v[range_max_idxs] - v[range_min_idxs]) | ||
|
||
`LibV5`_ : AP_phaseslope_AIS | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Same as AP_phaseslope, but for AIS location | ||
|
||
Please, notice that you have to provide t, v, stim_start and stim_end for location. | ||
|
||
- **Required features**: T;location_AIS, V;location_AIS, stim_start;location_AIS, stim_end;location_AIS, LibV5:AP_begin_indices;location_AIS | ||
- **Parameters**: AP_phaseslope_range | ||
- **Units**: 1/(ms) | ||
- **Pseudocode**: :: | ||
|
||
range_max_idxs = AP_begin_indices + AP_phseslope_range | ||
range_min_idxs = AP_begin_indices - AP_phseslope_range | ||
AP_phaseslope_AIS = (dvdt[range_max_idxs] - dvdt[range_min_idxs]) / (v[range_max_idxs] - v[range_min_idxs]) | ||
|
||
`LibV5`_ : BPAPHeightLoc1 | ||
~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Voltage height (difference betwen peaks and voltage base) at dendrite location | ||
|
||
Please, notice that you have to provide t, v, stim_start and stim_end for location. | ||
|
||
- **Required features**: T;location_dend1, V;location_dend1, stim_start;location_dend1, stim_end;location_dend1, peak_voltage;location_dend1, voltage_base;location_dend1 | ||
- **Units**: mV | ||
- **Pseudocode**: :: | ||
|
||
BPAPHeightLoc1 = peak_voltage - voltage_base | ||
|
||
`LibV5`_ : BPAPHeightLoc2 | ||
~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Same as BPAPHeightLoc1, but for dend2 location | ||
|
||
- **Required features**: T;location_dend2, V;location_dend2, stim_start;location_dend2, stim_end;location_dend2, peak_voltage;location_dend2, voltage_base;location_dend2 | ||
- **Units**: mV | ||
- **Pseudocode**: :: | ||
|
||
BPAPHeightLoc2 = peak_voltage - voltage_base | ||
|
||
`LibV5`_ : BPAPAmplitudeLoc1 | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Amplitude at dendrite location | ||
|
||
Please, notice that you have to provide t, v, stim_start and stim_end for location. | ||
|
||
- **Required features**: T;location_dend1, V;location_dend1, stim_start;location_dend1, stim_end;location_dend1, peak_voltage;location_dend1, AP_begin_voltage;location_dend1 | ||
- **Units**: mV | ||
- **Pseudocode**: :: | ||
|
||
BPAPAmplitudeLoc1 = peak_voltage - AP_begin_voltage | ||
|
||
`LibV5`_ : BPAPAmplitudeLoc2 | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Same as BPAPAmplitudeLoc1, but for dend2 location | ||
|
||
- **Required features**: T;location_dend2, V;location_dend2, stim_start;location_dend2, stim_end;location_dend2, peak_voltage;location_dend2, AP_begin_voltage;location_dend2 | ||
- **Units**: mV | ||
- **Pseudocode**: :: | ||
|
||
BPAPAmplitudeLoc2 = peak_voltage - AP_begin_voltage | ||
|
||
`LibV5`_ : BAC_width | ||
~~~~~~~~~~~~~~~~~~~~ | ||
|
||
AP width at epsp location | ||
|
||
Please, notice that you have to provide t, v, stim_start and stim_end for location. | ||
|
||
- **Required features**: T;location_epsp, V;location_epsp, stim_start;location_epsp, stim_end;location_epsp, AP_width;location_epsp | ||
- **Units**: ms | ||
- **Pseudocode**: :: | ||
|
||
BAC_width = AP_width | ||
|
||
`LibV5`_ : BAC_maximum_voltage | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Maximuum voltage at epsp location | ||
|
||
Please, notice that you have to provide t, v, stim_start and stim_end for location. | ||
|
||
- **Required features**: T;location_epsp, V;location_epsp, stim_start;location_epsp, stim_end;location_epsp, maximum_voltage;location_epsp | ||
- **Units**: mV | ||
- **Pseudocode**: :: | ||
|
||
BAC_maximum_voltage = maximum_voltage | ||
|
||
|
||
|
||
Voltage features | ||
---------------- | ||
|
||
|
@@ -2007,7 +1902,7 @@ Computes the impedance given a ZAP current input and its voltage response. | |
It will return the frequency at which the impedance is maximal, in the range (0, impedance_max_freq] Hz, | ||
with impedance_max_freq being a setting with 50.0 as a default value. | ||
|
||
- **Required features**: current, LibV1:Spikecount, LibV5:voltage_base, LibV5:current_base | ||
- **Required features**: current, spike_count, LibV5:voltage_base, LibV5:current_base | ||
- **Units**: Hz | ||
- **Pseudocode**: :: | ||
|
||
|
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please tell in the note that this feature replaces Spikecount, that Spikecount is deprecated, but can still be used for the moment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, I also noticed I forgot to update this one. Done in the last commit. 👍