Add helper function GetWaveDimensionality and use it throughout codebase#2686
Add helper function GetWaveDimensionality and use it throughout codebase#2686
Conversation
|
@copilot Looks good so far. Now use the new function in the appropriate places all over the code base. A first try would be to have a look at: |
... Applied
|
|
@copilot The tests for GetWaveDimensionality are missing the case where the input wave is completely empty. |
|
@copilot The function |
c42ca7b to
4a5b932
Compare
Agent-Logs-Url: https://github.com/AllenInstitute/MIES/sessions/b10fcbd6-09e7-4c61-a617-c02789769fb6 Agent-Logs-Url: https://github.com/AllenInstitute/MIES/sessions/ce13e3dd-2eb2-460b-8efc-8e9532fafd24 Agent-Logs-Url: https://github.com/AllenInstitute/MIES/sessions/c0d50d5b-2fe0-4218-8f24-d38ba2f0cd46 Co-authored-by: t-b <1185677+t-b@users.noreply.github.com>
Agent-Logs-Url: https://github.com/AllenInstitute/MIES/sessions/c9c9143a-f661-4379-8487-0c45d58a5f7e Co-authored-by: t-b <1185677+t-b@users.noreply.github.com>
Agent-Logs-Url: https://github.com/AllenInstitute/MIES/sessions/c9c9143a-f661-4379-8487-0c45d58a5f7e Co-authored-by: t-b <1185677+t-b@users.noreply.github.com>
We now use GetWaveDimensionality for checking N-D ness and that accepts single column waves. So let's remove that check.
4a5b932 to
585501f
Compare
|
Polished commits a bit and also added minor fixes. Will merge once CI passes. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a threadsafe helper (GetWaveDimensionality) to treat waves with trailing size-1 dimensions as effectively lower-dimensional, and adopts it across MIES to replace WaveDims/DimSize-based dimensionality checks that were producing incorrect results.
Changes:
- Added
GetWaveDimensionality(WAVE/Z wv)to determine the highest dimension withDimSize > 1(elseROWS). - Added a dedicated unit-test block covering 1D–4D cases, trailing-1 dimensions, empty waves, and threadsafe execution.
- Replaced many “is 1D?” assertions and several SweepFormula dimensionality guards throughout the codebase and updated the SweepFormula docs example.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Packages/tests/Basic/UTF_Utils_WaveHandling.ipf | Removes an outdated ZapNullRefs expectation and adds comprehensive GetWaveDimensionality tests. |
| Packages/MIES/MIES_Utilities_WaveHandling.ipf | Adds GetWaveDimensionality and updates multiple 1D precondition checks to use it. |
| Packages/MIES/MIES_Utilities_Conversions.ipf | Updates 1D-only assertion to use GetWaveDimensionality. |
| Packages/MIES/MIES_Utilities_Algorithm.ipf | Updates several 1D assertions (set ops, search helpers) to use GetWaveDimensionality. |
| Packages/MIES/MIES_ThreadsafeUtilities.ipf | Updates 1D assertion for varNames to use GetWaveDimensionality. |
| Packages/MIES/MIES_TestPulse.ipf | Updates data dimensionality assertion to use GetWaveDimensionality. |
| Packages/MIES/MIES_SweepFormula.ipf | Updates plotter text-wave dimensionality assertions to use GetWaveDimensionality. |
| Packages/MIES/MIES_SweepFormula_PSX.ipf | Updates “Expected 1D wave” assertion to use GetWaveDimensionality. |
| Packages/MIES/MIES_SweepFormula_Operations.ipf | Updates several operations’ “up to 2D” guards to use GetWaveDimensionality <= COLS. |
| Packages/MIES/MIES_SweepFormula_Operations_Select.ipf | Updates selsweeps argument dimensionality assertion to use GetWaveDimensionality. |
| Packages/MIES/MIES_SweepFormula_Helpers.ipf | Adjusts scalar extraction checks (now using numpnts==1 in two places) and updates an epoch-pattern 1D check to use GetWaveDimensionality. |
| Packages/MIES/MIES_SweepFormula_Executor.ipf | Updates the 4D subarray guard to use GetWaveDimensionality. |
| Packages/MIES/MIES_MiesUtilities_Algorithm.ipf | Updates dims-wave 1D assertion in GetNumberOfUsefulThreads to use GetWaveDimensionality. |
| Packages/MIES/MIES_JSONWaveNotes.ipf | Updates 1D wave-ref-wave assertion to use GetWaveDimensionality. |
| Packages/MIES/MIES_ExperimentDocumentation.ipf | Updates “values must be 1D” assertion to use GetWaveDimensionality. |
| Packages/MIES/MIES_DataConfigurator.ipf | Updates testpulse dimensionality assertions to use GetWaveDimensionality. |
| Packages/doc/SweepFormula.rst | Updates documentation example to use GetWaveDimensionality for the 2D limit check. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Thanks for opening a PR in MIES ✨!
here for the detailed explanation.
Description
WaveDimsreturns dimension count regardless of trailing size-1 dimensions:Adds
GetWaveDimensionality(wv)— a threadsafe function that returns the highest dimension with size > 1:Changes
GetWaveDimensionalityfunction withWAVE/Zparameter to allow null wave referencesWaveDims(input) <= 2→GetWaveDimensionality(input) <= COLS)WaveDims(wv) > 1→GetWaveDimensionality(wv) > ROWS)GetWaveDimensionality(wv) == ROWS📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.
Close #2138