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.

Sign up for GitHub

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

Jump to bottom

SF: Adapt SFH_EvaluateRange for multi dataset input #1987

Merged
merged 20 commits into from
Feb 15, 2024
Merged

SF: Adapt SFH_EvaluateRange for multi dataset input #1987

merged 20 commits into from
Feb 15, 2024

Conversation

MichaelHuth
Copy link
Collaborator

@MichaelHuth MichaelHuth commented Jan 20, 2024 •
edited by t-b
Loading

  • SFH_EvaluateRange did not accept multi dataset input, which prevented formulas like "epochs(ST) + [1, -2]" to work. This functionality is added.
  • Changed SFH_GetSweepsForFormula to accept dataset input only.
  • Adapted calling sites.

close #1819

  • Tests / adapt Tests
  • PSX changes
  • Documentation
  • Handle non-existing (null wave ref) numeric range from epochs in SFH_GetSweepsForFormula graciously

@MichaelHuth MichaelHuth self-assigned this Jan 20, 2024
@MichaelHuth MichaelHuth force-pushed the feature/1987-sf_allow_multi_data_range_argument branch from acd7dd0 to 5c3642d Compare January 23, 2024 18:27
@MichaelHuth MichaelHuth assigned t-b and unassigned MichaelHuth Jan 23, 2024
@t-b

This comment was marked as outdated.

Sign in to view
@t-b

This comment was marked as outdated.

Sign in to view
@t-b

This comment was marked as outdated.

Sign in to view
@t-b

This comment was marked as outdated.

Sign in to view
@t-b
Copy link
Collaborator

t-b commented Jan 29, 2024 •
edited
Loading

  • SFH_Assert in psxStats that no two ranges (numeric and epoch) from a sweep intersect

@t-b t-b force-pushed the feature/1987-sf_allow_multi_data_range_argument branch 2 times, most recently from d28b4e0 to ae4b8e8 Compare February 5, 2024 23:12
@t-b t-b assigned MichaelHuth and unassigned t-b Feb 5, 2024
@t-b
Copy link
Collaborator

t-b commented Feb 5, 2024

@MichaelHuth Please have a look. I've also changed your commits to better fit the new story.

@t-b t-b assigned t-b and unassigned MichaelHuth Feb 6, 2024
@t-b

This comment was marked as outdated.

Sign in to view
@t-b t-b force-pushed the feature/1987-sf_allow_multi_data_range_argument branch 2 times, most recently from 3eddef3 to 7a4e7ee Compare February 6, 2024 16:49
@t-b t-b assigned MichaelHuth and unassigned t-b Feb 7, 2024
MichaelHuth
MichaelHuth commented Feb 9, 2024
View reviewed changes
Copy link
Collaborator Author

@MichaelHuth MichaelHuth left a comment

Choose a reason for hiding this comment

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

See comments

@MichaelHuth MichaelHuth assigned t-b and unassigned MichaelHuth Feb 9, 2024
t-b and others added 4 commits February 12, 2024 19:11
@t-b
SplitWavesToDimension: Add it
4dffb5b
@t-b
IsDoubleFloatingPointWave/IsSingleFloatingPointWave: Add them
bc8d2a6
@t-b
Tests/Test_WaveRefNumeric(): Enhance them
81a6be8 
Make the wave sizes and contents assymetric so that wrong row/column major
order issues would be found.
@MichaelHuth @t-b
SF: Adapt SFH_EvaluateRange for multi dataset input
422eac8 
- SFH_EvaluateRange did not accept multi dataset input,
  which prevented formulas like "epochs(ST) + [1, -2]" to work.
  This functionality is added.
- Changed SFH_GetSweepsForFormula to accept dataset input only.
- Adapted calling sites.
- added tests as part of tests for data operation
	- also added test for (n, 2) numerical ranges
t-b added 16 commits February 12, 2024 19:13
@t-b
SF_GetTraceAnnotationText: Avoid duplicating code
909df2e
@t-b
SFH_GetSweepsForFormula: Ensure that all input ranges are double prec…
8f13c6f 
…ision

And fix call sites which don't fullfill that requirement.
@t-b
SF_OperationEpochs: Add operations stack support
bb67c9e 
This is nicer for the data as we then have sweep numbers in the
annotation and correct trace colors.
@t-b
TestOperationData: Ensure correct channels for fail tests
151fe3d 
We want to check that something else than the channel number fails. Bug
introduced in 365018e (SF: Add test for operation data to check for skip
of non-existing epochs, 2022-09-20) and 504ba60 (SF: Add Tests for data
operation, 2022-01-21).
@t-b
SF_OperationEpochs: Output grouped ranges
71f6358 
Since we added support for inputting multiple numeric ranges into data
the case of

sel=select(channels(AD9),[31,32,35,37,39],all)
data(epochs(["E0", "TP"] , $sel), $sel)

did not work correctly.

This was because epochs returns 10 waves for this case but we only have 5
sweeps. The information that we have two ranges per sweep was lost.

The epochs operation now outputs $numSweeps datasets were all ranges are
contained in a 2D wave.
@t-b
SFH_GetSweepsForFormula: Enforce that rangeStart < rangeEnd
416838c
@t-b
PSX_OperationKernel: Mention that this is not SF_META_RANGE
ccdb063
@t-b
MIES_SweepFormula_PSX.ipf: Add support for multiple ranges
595daef
@t-b
SFH_GetNumericRangeFromEpoch: Factor it out
88db009
@t-b
SFH_GetRangeFromEpoch: Simplify epoch name handling
637ed89 
We already have the epoch shortnames, with fallback to the tag if empty,
so we can just use these.
@t-b
SFH_GetNumericRangeFromEpoch: Rename epochNames
62c326a
@t-b
AreIntervalsIntersecting: Add it
429c81d
@t-b
PSX_OperationStatsImpl: Assert on intersecting ranges
6d10790
@t-b
Tests: Add some more for ZapNaNs
1fd930b
@t-b
tools/check-code.sh: Fix typo in error message
5c257e9
@t-b
tools/check-code.sh: Tighten function-call-in-for-loop check
c826f60 
We only want to match classical for loops with for(<>; <>; <>) and not
range-based for loops with for(<> : <>).
@t-b t-b force-pushed the feature/1987-sf_allow_multi_data_range_argument branch from 7a4e7ee to c826f60 Compare February 12, 2024 19:27
@t-b t-b assigned MichaelHuth and timjarsky and unassigned t-b Feb 12, 2024
@timjarsky timjarsky removed their assignment Feb 13, 2024
MichaelHuth
MichaelHuth commented Feb 15, 2024
View reviewed changes
Copy link
Collaborator Author

@MichaelHuth MichaelHuth 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 to me.

@MichaelHuth MichaelHuth assigned t-b and unassigned MichaelHuth Feb 15, 2024
@t-b t-b merged commit 9a4ef75 into main Feb 15, 2024
19 checks passed
@t-b t-b deleted the feature/1987-sf_allow_multi_data_range_argument branch February 15, 2024 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timjarsky timjarsky timjarsky approved these changes

@t-b t-b Awaiting requested review from t-b t-b is a code owner

Assignees

@t-b t-b

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow multi data input for range argument of data operation
3 participants
@MichaelHuth @t-b @timjarsky