-
Notifications
You must be signed in to change notification settings - Fork 9
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
Enhance adaptive suprathreshold #2302
Open
t-b
wants to merge
28
commits into
main
Choose a base branch
from
feature/2302-another-adaptive-revision
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
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
t-b
force-pushed
the
feature/2302-another-adaptive-revision
branch
from
November 27, 2024 22:53
daec7e1
to
886e9e6
Compare
t-b
force-pushed
the
feature/2302-another-adaptive-revision
branch
2 times, most recently
from
January 21, 2025 17:56
d7b9af8
to
7a4e927
Compare
t-b
force-pushed
the
feature/2302-another-adaptive-revision
branch
3 times, most recently
from
January 26, 2025 20:44
87bc99b
to
03ebd15
Compare
This comment was marked as outdated.
This comment was marked as outdated.
t-b
force-pushed
the
feature/2302-another-adaptive-revision
branch
2 times, most recently
from
January 27, 2025 18:48
c643b38
to
eaa0825
Compare
timjarsky
reviewed
Jan 27, 2025
…sweeps The new FailingAdaptiveSCIRange analysis parameter allows to tune the formerly hardcoded (1) number of SCIs we look back for failing adaptive sets. It defaults to 1, can be turned off with 0 and allows inf for always taking all SCIs into account.
We don't need the sub pA precision. In PSQ_DS_GatherOvershootCorrection we now also check if the calulated value is the same as the before/after value, as this can now happen with rounding. In PSQ_DS_CalculateDAScale we also use ceil for the too large/small cases so that we always get a different value.
We now perform the already measured check only if we have a candidate at all.
Debugging the future DAScales labnotebook entries can be challenging as it is not obvious form where the values where added. We now fix that by adding a type to the futureDAScales waves which is then also stored in the labnotebook.
Both variants for the adaptive data and for RhSuAd.
For a future application we want to check for something different than equality, so making the predicate configurable here is required.
…PSQ_FMT_LBN_SWEEP_PASS This has a different explanation but in this sense it is never used. Introduced in c53b5d0 (PSQ_DaScale: Enhance adaptive mode, 2024-07-26).
Having a duplicate version for PSQ_DAScale Adaptive makes it easier to check if we have all variants (offsets, RhSuAd, DAScale, etc.) properly done.
…out baseline QC Needed for a future application.
The stock Igor Pro functions don't maintain the relative order of the same key values. So we had to roll our own.
…terFrequencyCurrentData
In that way this is not dead code in the tests. It is also clearer that we check this error condition first.
- Ignore sweeps with negative fI slopes for the DAScale estimation - Store a new QC value if the fI slope is finite and negative - Using this new QC we are now also done if: - We have to consecutive negative fI slope values (as usual failing sweep QC sweeps can be in between) - or we have a passing fI slope and a consecutive (as above) negative fI slope value If we finish due to these two conditions we also fillin another DAScale in between the positive FI slope sweep and the first negative fI slope value. - We now use the new analysis parameter DAScaleNegativeSlopePercent, defaulting to 50%, to calculate a new DAScale value after a negative slope. We use the percentage of the maximum DAScale step for that. - Add a new labnotebook entry which denotes if a value is filled in or not - Also a bug is fixed which did not ignore fillin sweeps when trying to determine if the set is passing. - And similiar bug could result in fillin sweeps being marked as fI slope passing.
In (PSQ_DAScale: Change fitting logic and drop NumPointsForLineFit, 2024-07-10) we added dropping duplicated AP frequencies in the input data for fitting. We do catch that now with the neg fI slope QC better. And that also removes the hack to find a good DAScale in case this was filtered out.
t-b
force-pushed
the
feature/2302-another-adaptive-revision
branch
2 times, most recently
from
January 28, 2025 14:38
00b6e6f
to
ae1e170
Compare
@timjarsky Ready to test. |
t-b
force-pushed
the
feature/2302-another-adaptive-revision
branch
from
January 28, 2025 14:45
ae1e170
to
17d9733
Compare
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.
Close #2258