Conversation
75c4b24 to
bd0beed
Compare
|
@timjarsky This is a first version to play around. There are still a few things I have to add, that I discuss farther below.
xaxisOffset and yaxisOffset are strings that can be General Plotting BehaviorThe operation itself returns internally a full plotting specification that is inserted by the formula plotter at the location where the operation appears in the notebook code. The operation creates only traces that are separated by
Thus, the plotter applies the 10% for x and y-axis when used like this, where the formula setting the plot properties is last in the chain: but not for this: because Operation ArgumentsCurrently:
with The (later) final arguments should also expose arguments from The default for On the basis of the experiment avgMethodTesting2.pxp the generated code is: I added a
for
for I need to add a Therefore, the An additional task from the issue is to add a variable that contains the names of the experiments. I can create this variable in the operation and add it to the variable storage of the formula notebook. It would be available then after the operation ran. |
|
@MichaelHuth attempting to use ivscc_apfrequency() on the linked data , resulted in the following assertions: |
|
I had to fill a request form from Google for the data. |
bd0beed to
d228f41
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a new ivscc_apfrequency operation to the sweep formula system, along with supporting infrastructure changes including a flatten operation and refactored plotting code.
Key Changes
- Adds
ivscc_apfrequencyoperation for analyzing action potential frequency in IV sweep current clamp experiments - Introduces
flattenhelper operation to convert datasets of single values into 1D arrays - Refactors plotting infrastructure by introducing
SF_PlotterGraphStructto encapsulate plotting state - Adds axis offset and percentage control capabilities to the plotting metadata system
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| Packages/MIES/MIES_Constants.ipf | Adds new metadata constants for plot customization and operation names (contains merge conflict) |
| Packages/MIES/MIES_SweepFormula_Operations.ipf | Implements flatten and ivscc_apfrequency operations |
| Packages/MIES/MIES_SweepFormula_Executor.ipf | Registers new operations in the executor switch statement |
| Packages/MIES/MIES_SweepFormula.ipf | Major refactoring of plotting code to use structure-based state management |
| Packages/MIES/MIES_SweepFormula_Helpers.ipf | Adds helper functions for internal formula execution and variable storage |
| Packages/MIES/MIES_WaveDataFolderGetters.ipf | Expands plot metadata wave to include axis offset/percentage properties |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d228f41 to
5e60339
Compare
|
@timjarsky I fixed the assertion that came up. The reason was that with the three experiments the selections result in 20 sweeps from the first experiment, 24 from the second and 16 from the third. The avg operation was transferring the wave notes and meta data always from the first group to the averaged result. And this failed because the averaged result always has the highest number of datasets (24 in this case) and I can not transfer from 20 datasets to 24 datasets as for the last 4 there would be not wave notes and meta data to transfer. For this data this also means that for the first 16 sweeps in the groups the average is over 3 sweeps, 16 - 19 over 2 sweeps and 20 - 23 over 1 sweep. In the most recent version of this PR also the And the |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 14 comments.
Comments suppressed due to low confidence (1)
Packages/MIES/MIES_SweepFormula.ipf:1630
- Missing return statement: The function SF_AddPlotTraceStyle declares a return value of type SF_PlotterGraphStruct but has no explicit return statement. This will cause a compilation error or undefined behavior. Add 'return [pg]' at the end of the function.
static Function [STRUCT SF_PlotterGraphStruct pg] SF_AddPlotTraceStyle(variable formulasAreDifferent)
variable i, j, numTraces, markerCode, lineCode, isCategoryAxis, tagCounter, lineStyle, overrideMarker, traceToFront
string trace, info, tagText, name, wvName
for(i = 0; i < pg.formulaCounter; i += 1)
WAVE/WAVE plotFormData = pg.collPlotFormData[i]
WAVE/T tracesInGraph = plotFormData[0]
WAVE/WAVE dataInGraph = plotFormData[1]
numTraces = DimSize(tracesInGraph, ROWS)
markerCode = formulasAreDifferent ? i : 0
markerCode = SFH_GetPlotMarkerCodeSelection(markerCode)
lineCode = formulasAreDifferent ? i : 0
lineCode = SFH_GetPlotLineCodeSelection(lineCode)
for(j = 0; j < numTraces; j += 1)
WAVE/Z wvX = dataInGraph[j][%WAVEX]
WAVE wvY = dataInGraph[j][%WAVEY]
trace = tracesInGraph[j]
info = AxisInfo(pg.win, "left")
isCategoryAxis = (NumberByKey("ISCAT", info) == 1)
if(isCategoryAxis)
WAVE traceColorHolder = wvX
else
WAVE traceColorHolder = wvY
endif
WAVE/Z traceColor = JWN_GetNumericWaveFromWaveNote(traceColorHolder, SF_META_TRACECOLOR)
if(WaveExists(traceColor))
switch(DimSize(traceColor, ROWS))
case 3:
ModifyGraph/W=$pg.win rgb($trace)=(traceColor[0], traceColor[1], traceColor[2])
break
case 4:
ModifyGraph/W=$pg.win rgb($trace)=(traceColor[0], traceColor[1], traceColor[2], traceColor[3])
break
default:
FATAL_ERROR("Invalid size of trace color wave")
endswitch
endif
tagText = JWN_GetStringFromWaveNote(wvY, SF_META_TAG_TEXT)
if(!IsEmpty(tagText))
name = "tag" + num2str(tagCounter++)
Tag/C/N=$name/W=$pg.win/F=0/L=0/X=0.00/Y=0.00 $trace, 0, tagText
endif
ModifyGraph/W=$pg.win mode($trace)=SF_DeriveTraceDisplayMode(wvX, wvY)
lineStyle = JWN_GetNumberFromWaveNote(wvY, SF_META_LINESTYLE)
if(IsValidTraceLineStyle(lineStyle))
ModifyGraph/W=$pg.win lStyle($trace)=lineStyle
elseif(formulasAreDifferent)
ModifyGraph/W=$pg.win lStyle($trace)=lineCode
endif
WAVE/Z customMarkerAsFree = JWN_GetNumericWaveFromWaveNote(wvY, SF_META_MOD_MARKER)
if(WaveExists(customMarkerAsFree))
DFREF dfrWork = SFH_GetWorkingDF(pg.graph)
wvName = "customMarker_" + NameOfWave(wvY)
WAVE customMarker = MoveFreeWaveToPermanent(customMarkerAsFree, dfrWork, wvName)
ASSERT(DimSize(wvY, ROWS) == DimSize(customMarker, ROWS), "Marker size mismatch")
ModifyGraph/W=$pg.win zmrkNum($trace)={customMarker}
else
overrideMarker = JWN_GetNumberFromWaveNote(wvY, SF_META_MOD_MARKER)
if(!IsNaN(overrideMarker))
markerCode = overrideMarker
endif
ModifyGraph/W=$pg.win marker($trace)=markerCode
endif
traceToFront = JWN_GetNumberFromWaveNote(wvY, SF_META_TRACETOFRONT)
traceToFront = IsNaN(traceToFront) ? 0 : !!traceToFront
if(traceToFront)
ReorderTraces/W=$pg.win _front_, {$trace}
endif
endfor
endfor
End
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0b71dcf to
dc98aba
Compare
|
Thanks for handling the metadata management for mismatched sweep numbers across experiments. A few points for discussion:
|
dc98aba to
79f149a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
79f149a to
95732dc
Compare
|
@timjarsky I have added support for the apfrequency argument block after the first four argument for ivscc_apfrequency. The arguments are now: The last four arguments are "forwarded" to apfrequency. |
|
@MichaelHuth, Are failing sweeps included or filtered out? If included, please update to use only passing sweeps. |
By definition the first NUM_HEADSTAGES colors are used to distinguish data from headstages. The following color is used for averaged traces. The new function GetTraceColorNonHeadstage returns a color that is not part of this pool. Also fixed a typo in the description for GetTraceColorForAverage
…FitFunctionCoefficientNumber The functions allow to programmatically retrieve information about Igors integrated fit functions. IsIntegratedFitFunction allows to check a fit function name if it matches an Igor integrated fit function. GetIntegratedFitFunctionCoefficientNumber returns the number of coefficients required by an integrated fit function. The wave getter GetSFIgorFitProperties can be extended with more information in additional columns if needed.
This allows to specify fit constraints in the form "K0 > 3" in a sweep formula.
The PrepareFit operation allows to gather information for fitting input data. The function returns a wave reference wave that stores all gathered information. The format of this wave is designed to allow further extensions for e.g. masking, weights etc. This is a preparation operation for a fit operation that implements the actual fit. PrepareFit allows to use the same fit setup for different fits.
- this allows the user to use getmeta($ivscc_apfrequency_fit)
396c0a1 to
95edd75
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| WAVE wTmp = output[i] | ||
| printf "Bin: %d Avg result: %f, xValue: %f, ySdev: %f, xSdev: %f\r", i, wTmp[0], xValue, ySdev, xSdev |
There was a problem hiding this comment.
Debug print statements should be removed or placed behind a debug flag. These printf statements will clutter the history window in production use, which is not typical for production code in this codebase.
| WAVE wTmp = output[i] | |
| printf "Bin: %d Avg result: %f, xValue: %f, ySdev: %f, xSdev: %f\r", i, wTmp[0], xValue, ySdev, xSdev | |
| WAVE wTmp = output[i] | |
| #ifdef SWEEPFORMULA_DEBUG | |
| printf "Bin: %d Avg result: %f, xValue: %f, ySdev: %f, xSdev: %f\r", i, wTmp[0], xValue, ySdev, xSdev | |
| #endif |
| printf "avg in bins mode:\r" | ||
|
|
||
| [s] = GetTraceColorForAverage() | ||
| Make/FREE/W/U traceColor = {s.red, s.green, s.blue} | ||
|
|
||
| numGroups = DimSize(input, ROWS) | ||
| binStart = binRange[0] | ||
| binEnd = binRange[1] | ||
| numBins = ceil((binEnd - binStart) / binWidth) | ||
| SFH_ASSERT(numBins < 1E6, "Maximum number of bins is 1E6.") | ||
|
|
||
| // Gather | ||
| Make/FREE/WAVE/N=(numBins, numGroups) binnedPerGroup | ||
| for(i = 0; i < numGroups; i += 1) | ||
| WAVE/WAVE dataSets = input[i] | ||
| WAVE/WAVE binDataSets = binData[i] | ||
| numDataSets = DimSize(dataSets, ROWS) | ||
| SFH_ASSERT(numDataSets == DimSize(binDataSets, ROWS), "The number of datasets of the input and bins are not the same for group " + num2istr(i)) | ||
| for(j = 0; j < numDataSets; j += 1) | ||
| if(!WaveExists(dataSets[j])) | ||
| continue | ||
| endif | ||
| SFH_ASSERT(WaveExists(binDataSets[j]), "A bin dataset is null") | ||
| SFH_ASSERT(IsNumericWave(binDataSets[j]), "A bin dataset must be numeric") | ||
| SFH_ASSERT(DimSize(binDataSets[j], ROWS) == 1, "A bin dataset must have exactly one value") | ||
| binValue = WaveRef(binDataSets, row = j)[0] | ||
| if(binValue < binStart || binValue >= binEnd) | ||
| continue | ||
| endif | ||
| binPos = floor((binValue - binStart) / binWidth) | ||
| // Add to bin | ||
| WAVE/Z/WAVE wavesInBin = binnedPerGroup[binPos][i] | ||
| if(!WaveExists(wavesInBin)) | ||
| Make/FREE/WAVE wavesInBin = {dataSets[j]} | ||
| SetNumberInWaveNote(wavesInBin, NOTE_INDEX, 1) | ||
| binnedPerGroup[binPos][i] = wavesInBin | ||
| continue | ||
| endif | ||
| idx = GetNumberFromWaveNote(wavesInBin, NOTE_INDEX) | ||
| EnsureLargeEnoughWave(wavesInBin, indexShouldExist = idx) | ||
| wavesInBin[idx] = dataSets[j] | ||
| SetNumberInWaveNote(wavesInBin, NOTE_INDEX, idx + 1) | ||
| endfor | ||
| endfor | ||
| printf "Averaging bins:\r" | ||
| // avg bins with multiple filling | ||
| for(i = 0; i < numBins; i += 1) | ||
| for(j = 0; j < numGroups; j += 1) | ||
| if(!WaveExists(binnedPerGroup[i][j])) | ||
| continue | ||
| endif | ||
| WAVE/WAVE wavesInBin = binnedPerGroup[i][j] | ||
| idx = GetNumberFromWaveNote(wavesInBin, NOTE_INDEX) | ||
| Redimension/N=(idx) wavesInBin | ||
| printf "Bin %d, Group %d, NumWaves %d, ", i, j, idx | ||
| if(idx == 1) | ||
| WAVE wTmp = wavesInBin[0] | ||
| print "Result:", wTmp[0] | ||
|
|
||
| continue | ||
| endif | ||
| WAVE/WAVE avg = MIES_fWaveAverage(wavesInBin, 1, IGOR_TYPE_64BIT_FLOAT) | ||
| Redimension/N=(1) wavesInBin | ||
| wavesInBin[0] = avg[0] | ||
|
|
||
| WAVE wTmp = avg[0] | ||
| print "Result:", wTmp[0] | ||
|
|
||
| endfor | ||
| endfor | ||
| // avg same bins | ||
| WAVE/WAVE output = SFH_CreateSFRefWave(graph, opShort, numBins) | ||
| for(i = 0; i < numBins; i += 1) | ||
| Make/FREE/WAVE/N=(numGroups) sameBin | ||
| idx = 0 | ||
| for(j = 0; j < numGroups; j += 1) | ||
| if(!WaveExists(binnedPerGroup[i][j])) | ||
| continue | ||
| endif | ||
| WAVE/WAVE wavesInBin = binnedPerGroup[i][j] | ||
| sameBin[idx] = wavesInBin[0] | ||
| idx += 1 | ||
| endfor | ||
| printf "Bin %d, NumWaves/TotalGroups %d/%d:\r", i, idx, numGroups | ||
| if(idx == 0) | ||
| Make/FREE/D tmp = {NaN} | ||
| output[i] = tmp | ||
| else | ||
| Redimension/N=(idx) sameBin | ||
| WAVE/WAVE avg = MIES_fWaveAverage(sameBin, 1, IGOR_TYPE_64BIT_FLOAT) | ||
| output[i] = avg[0] | ||
|
|
||
| WAVE wTmp = output[i] | ||
| printf "Avg result: %f\r", wTmp[0] |
There was a problem hiding this comment.
Debug print statements should be removed or placed behind a debug flag. These printf statements will clutter the history window in production use.
| if(FitError == 0) | ||
| msg += "Fit Ok.\r" | ||
| endif | ||
| if((FitError & FIT_ERROR_ANY) == FIT_ERROR_ANY) | ||
| msg += "Fit Error.\r" | ||
| endif | ||
| if((FitError & FIT_ERROR_SINGULARMATRIX) == FIT_ERROR_SINGULARMATRIX) | ||
| msg += "Singular Matrix.\r" | ||
| endif | ||
| if((FitError & FIT_ERROR_OUTOFMEMORY) == FIT_ERROR_OUTOFMEMORY) | ||
| msg += "Out of memory.\r" | ||
| endif | ||
| if((FitError & FIT_ERROR_RETURNEDNANORINF) == FIT_ERROR_RETURNEDNANORINF) | ||
| msg += "Fit function returned NaN or Inf.\r" | ||
| endif | ||
| if((FitError & FIT_ERROR_FUNCREQUESTEDSTOP) == FIT_ERROR_FUNCREQUESTEDSTOP) | ||
| msg += "Function requested stop.\r" | ||
| endif | ||
| if((FitError & FIT_ERROR_REENTRANT_FIT) == FIT_ERROR_REENTRANT_FIT) | ||
| msg += "Reentrant fit function call encountered.\r" | ||
| endif | ||
| if((FitQuitReason & FIT_QUITREASON_ITERATIONLIMITREACHED) == FIT_QUITREASON_ITERATIONLIMITREACHED) | ||
| msg += "Iteration limit reached.\r" | ||
| endif | ||
| if((FitQuitReason & FIT_QUITREASON_STOPPEDBYUSER) == FIT_QUITREASON_STOPPEDBYUSER) | ||
| msg += "User stopped fit.\r" | ||
| endif | ||
| if((FitQuitReason & FIT_QUITREASON_NOCHISQUAREDECREASE) == FIT_QUITREASON_NOCHISQUAREDECREASE) | ||
| msg += "chiSquare did not decrease in last 9 iterations.\r" | ||
| endif | ||
| msg += "Iterations run: " + num2istr(fitNumIters) |
There was a problem hiding this comment.
Parameter name casing inconsistency: the function parameter is fitError (lowercase) but the code references FitError (capitalized). In Igor Pro, variables are case-insensitive, but this inconsistency reduces code readability and violates the apparent naming convention where parameters use camelCase.
| numExp = DimSize(uniqueFiles, ROWS) | ||
| SFH_ASSERT(numExp > 0, "ivscc_apfrequency: data from at least one experiment has to be loaded") | ||
|
|
||
| formula = "sel = select(selsweeps(), selstimset(\"*LP_Rheo*\", \"*supra*\"), selvis(all), selivsccsweepqc(passed))\r" |
There was a problem hiding this comment.
The stimulus set pattern "LP_Rheo" is hardcoded into the ivscc_apfrequency operation. This reduces flexibility as it couples the operation to a specific naming convention for stimulus sets. Consider making this configurable through an optional parameter to allow users to specify their own stimulus set patterns.
95edd75 to
13a200c
Compare
|



close #2581
close #2628