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

SweepFormula/psxKernel: Various fixes #2023

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

t-b
Copy link
Collaborator

@t-b t-b commented Feb 23, 2024

@timjarsky This has our bugfixes for psxKernel and too large decayTau. I've removed the adaptations for the sweep range extraction as #1990 will fix that in a better way (also for old data).

  • Add test
  • Drop empty results in PSX_Operation
  • intersecting ranges bug IVSCCMiniAnalysis on the FTP (same name, new content)
  • StatsRangeTesting: Check that the correct range was used for multi dataset examples
  • Update psxStats documentation
  • Throw away events which have an amplitude which has a different sign compared to the psxKernel amplitude
  • Replace PSX_DEFAULT_X_START_OFFSET with a factor and the decay tau from psxKernel
  • Fix incorrect usage of StatsQuantiles as /Z with err = ... does not make sense
  • psx should return sweepData waves if there are no events, as psxPrep relies on that for the histogram. Otherwise the data psx uses and what psxPrep plots is different
  • Fix kernelAmp sign check again
  • Reverse order of offsetting and filtering, rename all names

Close #2016
Close #2011
Close #1660

@t-b t-b requested a review from timjarsky as a code owner February 23, 2024 14:58
@t-b t-b mentioned this pull request Feb 23, 2024
1 task
@t-b t-b assigned t-b and unassigned timjarsky Feb 23, 2024
@t-b t-b changed the title SweepFormula/psxKernel: Skip kernel creation for too large decayTau SweepFormula/psxKernel: Various fixes Feb 23, 2024
@t-b t-b assigned timjarsky and unassigned t-b Feb 23, 2024
@timjarsky timjarsky assigned t-b and unassigned timjarsky Feb 23, 2024
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from 718e3ac to 108f961 Compare March 1, 2024 18:24
@t-b t-b assigned timjarsky and unassigned t-b Mar 1, 2024
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from 108f961 to ed6c2fb Compare March 1, 2024 22:19
@timjarsky timjarsky assigned t-b and unassigned timjarsky Mar 7, 2024
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from ed6c2fb to 56305c1 Compare March 8, 2024 18:50
@t-b t-b mentioned this pull request Mar 13, 2024
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from 56305c1 to 475065e Compare April 18, 2024 19:36
@t-b t-b marked this pull request as draft April 19, 2024 18:17
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from 475065e to ffeff20 Compare May 21, 2024 18:15
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from ffeff20 to ffe3676 Compare June 5, 2024 22:13
@t-b t-b assigned timjarsky and unassigned t-b Jun 5, 2024
@t-b

This comment was marked as outdated.

@t-b t-b marked this pull request as ready for review June 5, 2024 22:13
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from ffe3676 to 7637e16 Compare June 5, 2024 22:38
@t-b t-b mentioned this pull request Jun 6, 2024
14 tasks
@timjarsky

This comment was marked as outdated.

The additional /Z in the range based for loop is not required, but that is
a bug reported to WM.

Forgotten in 422eac8 (SF: Adapt SFH_EvaluateRange for multi dataset
input, 2024-01-20).
We are now removing empty entries in the output wave. This avoids an issue
with psxPrep which did not anticipate that.

As we are now not feeding in any null waves into PSX_OperationImpl we can
also remove the checks there.
The current implementation for the ranges did not handle multiple passed
epochs nor epoch wildcards. The check for intersecting ranges added in
6d10790 (PSX_OperationStatsImpl: Assert on intersecting ranges,
2024-02-06) was also broken.

While thinking about the current approach it was recognized that we
actually want to collect the event data from all sweep ranges of all
sweeps from the same equivalence class.

This has now been done. We also support list of epochs and epoch
wildcards.
This prepares for a future commit where we throw away single events and
thus PSX_AnalyzePeaks can result in no events event if we found peaks in
PSX_FindPeaks.
In a future commit we want to reuse these calculations for event
filtering.
We know how many entries in psxEvent and eventFit we have before starting
the loop so we can just call Redimension before and are done.
… amplitude

We need to filter that out first as we need the neighbouring distance to
the passing events.

And we also have to introduce an override setting as otherwise the tests
don't pass.
We only use the fixed value in case tau is NaN.
We can be called from PSX_PlotInteractionHook where we don't know if the
passed index is valid.

Let's do nothing if the index is out of range.
In dc9ed87 (SF_PreparePlotter: Don't kill and recreate the main panel
for SF_DM_SUBWINDOWS, 2024-06-19) we switched from killing the
sweepformula plot to clearing it.

But we forgot to remove additional controls and draw elements added by
e.g. PSX.

And now we also have to explicitly remove TraceUserData (TUD) as these are
not cleaned up automatically anymore, as killing an embedded subwindow
does not trigger the main window hook.
Due to the changed window handling in dc9ed87 (SF_PreparePlotter: Don't
kill and recreate the main panel for SF_DM_SUBWINDOWS, 2024-06-19) we need
to ensure that the host window of the SF panel is active.
This makes it easier to use and is more according to our philosopy that null
wave refs should be preferred over empty waves.
We only need index in the else branch and some more spaces are nice.
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from f54b1b1 to eadd32d Compare September 6, 2024 15:04
@t-b
Copy link
Collaborator Author

t-b commented Sep 6, 2024

I've fixed the cursor issue.

@MichaelHuth FYI: 7e0c17e (DeepCopyWaveRefWave: Allow src to cotain null waves, 2024-09-06)

@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from eadd32d to 33e101c Compare September 6, 2024 15:10
Using psxPrep requires sweep data from psx. But we used to only return
sweep data if we have found events.

We now always return sweep data so that psxPrep always works.
We now first offset and then filter. This requires that we also rename all
variables, constants and dimension labels.

Change requested by Tim Jarsky.
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from 33e101c to 20d8f85 Compare September 6, 2024 21:47
@t-b t-b removed their assignment Sep 9, 2024
@t-b t-b assigned t-b and unassigned timjarsky Sep 10, 2024
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.

Make psx more robust psxStats does not support epoch wildcards Allow wildcard expressions in psxStats
2 participants