Add ivscc_apfrequency operation

[MichaelHuth]

close #2581

• Set the opacity of individual traces to 0.2. Leave average trace opacity at 1.
• adapt xvalues operation to extract also from xvalues meta data
• add support to plotter for opacity meta data
• Support fitting the data (use the error bars of the average trace to weight the fit). Consider implementing a new operation that's passed as an optional argument into ivscc_APfrequency.
• find out how to get automatically determined xOffset constant from e.g. exp_XOffset integrated fit func after fit -> W_fitConstants
• ??add argument to ivscc_apfrequency that enables that all experiments/cells are fitted.
• add argument to preparefit to limit the fitting range. For flexibility and simplicity I suggest to add this through a mask wave in fit2 then.
• Make the fit parameters accessible with a related operation
• ??the fit result may be based on the acquisition/sweep sequence rather than an ordered list of current intensity (x-scale values) -> 3rd avg mode?
• add a way to expose the number and type of required coefficients to the user. Perhaps a link to the relevant Igor documentation, and/or a pop-up? -> add to help of preparefit

TBs new todos:

• Data on the FTP for regression tests: pr-2599/inputRegressionTest
• C&P into Excel does not work as multiple entries are added into one cell
• Fix else // SF_OP_IVSCCAPFREQUENCY_NONE branches and add fatal error
• Add grouping option for the analysisbrowser (add option to generate SF code in the analysis browser for selected experiments, copy to clipboard via context menu, ivscc_apfrequency should support these named groups)
• getmeta
  • Tests
  • Docu
• fit2
  • Tests
  • Docu
• preparefit
  • Tests
  • Docu
• ivscc_apfrequency
  • Tests
  • Docu
• ihf docu

close #2628

[MichaelHuth]

@timjarsky This is a first version to play around. There are still a few things I have to add, that I discuss farther below.

ivscc_apfrequency([xaxisOffset, yaxisOffset, xAxisPercentage, yAxisPercentage]) where [] denotes that the parameters are optional.

xaxisOffset and yaxisOffset are strings that can be min, max and none.

General Plotting Behavior

The 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 with. This means preceding and succeeding formulas that are also separated with with go to the same subwindow.

xAxisPercentage and yAxisPercentage are treated as properties for the plot by the plotter. Specifically plot means in this context the subwindow where the traces go. Plot properties from the last formula in the with chain are applied. (i.e. there is currently no gathering of plot properties).

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:

1
with
ivscc_apfrequency(min, min, 10, 10)

but not for this:

ivscc_apfrequency(min, min, 10, 10)
with
1

because 1 does not include any plot properties.

Operation Arguments

Currently:

ivscc_apfrequency([xaxisOffset, yaxisOffset, xAxisPercentage, yAxisPercentage])

with xaxisOffset and yaxisOffset as min, max, none and xAxisPercentage, yAxisPercentage a number between 0 and 100.

The (later) final arguments should also expose arguments from apfrequency, so it will change to ivscc_apfrequency([xaxisOffset, yaxisOffset, xAxisPercentage, yAxisPercentage, method, level, resultType, normalize, xAxisType).

The default for xaxisOffset and yaxisOffset is min.

On the basis of the experiment avgMethodTesting2.pxp the generated code is:

sel = select(selsweeps(), selstimset("*rheo*", "*supra*"), selvis(all))
selexpAD0 = select(selexp("C57BL6J-734969.15.10A.01.nwb"), $sel, selchannels(AD0), selrange(E1))
selexpDA0 = select(selexp("C57BL6J-734969.15.10A.01.nwb"), $sel, selchannels(DA0), selrange(E1))
freq0 = apfrequency(data($selexpAD0))
current0 = max(data($selexpDA0))
currentNorm0 = $current0 - extract($current0, 0) #*1
selexpAD1 = select(selexp("C57BL6J-734969.15.10B.01.nwb"), $sel, selchannels(AD0), selrange(E1))
selexpDA1 = select(selexp("C57BL6J-734969.15.10B.01.nwb"), $sel, selchannels(DA0), selrange(E1))
freq1 = apfrequency(data($selexpAD1))
current1 = max(data($selexpDA1))
currentNorm1 = $current1 - extract($current1, 0) #*2
ivsccavg = avg([$freq0,$freq1], group)
ivscccurrentavg = avg([$currentNorm0,$currentNorm1], group)

$freq0 - extract($freq0, 0) #*3
vs
$currentNorm0
with
$freq1 - extract($freq1, 0) #*4
vs
$currentNorm1
with
$ivsccavg - extract($ivsccavg, 0) #*5
vs
$ivscccurrentavg - extract($ivscccurrentavg, 0) #*6

I added a #* to the formulas that change depending on the min, max, none setting.

#*1, #*2 and #*6 depend on the xaxisOffset argument with the following logic:

min: currentNormX = $currentX - extract($currentX, 0)
max: currentNormX = $currentX - max(flatten($currentX))
none: currentNormX = $currentX

for #*6 it is:
min: $ivscccurrentavg - extract($ivscccurrentavg, 0)
max: $ivscccurrentavg - max(flatten($ivscccurrentavg))
none: $ivscccurrentavg

#*3, #*4 and #*5 depend on the yaxisOffset argument with the following logic:

min: $freqX - extract($freqX, 0)
max: $freqX - max(flatten($freqX))
none: $freqX

for #*5 it is:
min: $ivsccavg - extract($ivsccavg, 0)
max: $ivsccavg - max(flatten($ivsccavg))
none: $ivsccavg

I need to add a flatten operation because the result in e.g. $freqX are single values in 7 datasets (for avgMethodTesting2.pxp). Thus, our max() operation would determine the max in each dataset individually, but that is not what is wanted. The flatten operation should change n datasets with a single data point to an array with n elements.

Therefore, the max argument is currently not implemented yet until I have the flatten operation implemented.

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.
For now I thought of $ivscc_apfrequency_explist as name and it would be a string array.

[timjarsky]

@MichaelHuth

Thanks for handling the metadata management for mismatched sweep numbers across experiments.

A few points for discussion:

1. The marker coloring makes it hard for me to evaluate the output (see plot image below). Can we use a single color for each experiment? Can the data be a single 1D wave instead of multiple x-y waves (this will enable fitting)?

2. There are many zero-frequency measurements spread across the x-axis.

3. The average seems more variable than the input data, perhaps because there are too many data points?

4. I'm not sure about the utility of the negative current values with "min" and "max" (second image). The two axes options that I'm sure are needed are none (where each FI curve starts at zero current and zero frequency) and the absolute current values.

ivscc_apfrequency(none, none)

ivscc_apfrequency()

[Copilot]

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.

[MichaelHuth]

@timjarsky

1. I changed that. There are now experiment + 1 traces with markers. The +1 trace is the average in the average color. The experiment traces use the common table for trace colors, with one color per experiment. I also added code to show the experiment name in the legend for these.
2. I still have to look into this, why a lot of the results return a zero. Currently the standard setting of apfrequency for the internal apfrequency call are used.
3. Yes, it looks a bit like that. I did not see that in my local testing with the three experiments though. I guess in the graphs you pasted there is for some of the average points only a single sweep for an index in the groups.
   e.g. first exp has 10 and second exp has 11 sweeps selected, then sweep data for index 10 is only present from the second experiment -> only one sweep goes into the average and then the trace point from the experiment and the trace point from the average are equal. The average is shown in front and covers the point from the experiment. You may check for the average points on the very top if there is a data point from an experiment underneath.
4. I think it is the other way around, with none no offset is applied. min moves the trace to zero and max moves the trace such that the former maximum point is at zero.
   What I think might be unexpected with the initial formula construct is that there is an offset used per trace. Thus, when changing none -> min then each trace start is moved to zero separately and the visual relation between the traces shifts. (same applies for max)
   This per-trace behavior is the same for the xaxisOffset and yaxisOffset.

[MichaelHuth]

@timjarsky
Regarding 2.: The default level for apfrequency is 0. This results in zero peaks found for sweeps where E1 does not cross the zero line and for this apfrequency returns 0 as result with the default arguments.

I have added support for the apfrequency argument block after the first four argument for ivscc_apfrequency. The arguments are now:
ivscc_apfrequency([xaxisOffset, yaxisOffset, xAxisPercentage, yAxisPercentage, method, level, resultType, normalize, xAxisType])

The last four arguments are "forwarded" to apfrequency.

[timjarsky]

@MichaelHuth, Are failing sweeps included or filtered out? If included, please update to use only passing sweeps.

[MichaelHuth]

• after JWs comment regarding CurveFit syntax, I am not sure if something for the "poly x" (and related) types need to be changed regarding extra arguments. Needs to be checked.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This test redeclares the local variable keys a second time (WAVE/Z/T keys = ...) in the same function scope, which is not allowed and will cause a compile error. Reuse the existing keys variable (assignment without redeclaration) or use a different variable name for the second call.

Suggested change

	WAVE/Z/T keys = JWN_GetKeys(wv, "I_DONT_EXIST")
	keys = JWN_GetKeys(wv, "I_DONT_EXIST")

[Copilot]

SF_OP_MERGE, SF_OP_FIT, SF_OP_FITLINE, and SF_OP_DATASET are defined twice in this block, which will cause a compile-time error due to duplicate StrConstant definitions. Remove the duplicated second set (lines 2577–2580) and keep only a single definition per operation constant.

Suggested change

	StrConstant SF_OP_MERGE = "merge"
	StrConstant SF_OP_FIT = "fit"
	StrConstant SF_OP_FITLINE = "fitline"
	StrConstant SF_OP_DATASET = "dataset"

[Copilot]

preparefit is documented/implemented to accept up to 5 arguments (including constraints at position 4), but SFH_CheckArgumentCount(..., maxArgs = 4) rejects any call that supplies the constraints argument. Update the max argument count to allow the constraints parameter.

Suggested change

	SFH_CheckArgumentCount(exd, opShort, 0, maxArgs = 4)
	SFH_CheckArgumentCount(exd, opShort, 0, maxArgs = 5)

[Copilot]

SFO_OperationGetMeta calls JWN_GetKeysAt(...), but there is no such function added/defined in the repo (the new helper is JWN_GetKeys). This will fail to compile; replace this call with the correct API (and handle the null-wave case if the path does not exist).

Suggested change

	WAVE/Z/T keys = JWN_GetKeysAt(data, SF_SERIALIZE)
	serStr = ""
	for(string key : keys)
	path = SF_SERIALIZE + "/" + key
	val = JWN_GetNumberFromWaveNote(data, path)
	if(!IsNaN(val))
	serStr += key + ": " + num2str(val, "%f") + "\r"
	continue
	endif
	str = JWN_GetStringFromWaveNote(data, path)
	if(!IsEmpty(str))
	serStr += key + ":\r" + str + "\r"
	continue
	endif
	WAVE/Z wvn = JWN_GetNumericWaveFromWaveNote(data, path)
	if(WaveExists(wvn))
	str = NumericWaveToList(wvn, "\r")
	serStr += key + ":\r" + str
	continue
	endif
	WAVE/Z/T wt = JWN_GetTextWaveFromWaveNote(data, path)
	if(WaveExists(wt))
	str = TextWaveToList(wt, "\r")
	serStr += key + ":\r" + str
	continue
	endif
	endfor
	WAVE/Z/T keys = JWN_GetKeys(data, SF_SERIALIZE)
	serStr = ""
	if(WaveExists(keys))
	for(string key : keys)
	path = SF_SERIALIZE + "/" + key
	val = JWN_GetNumberFromWaveNote(data, path)
	if(!IsNaN(val))
	serStr += key + ": " + num2str(val, "%f") + "\r"
	continue
	endif
	str = JWN_GetStringFromWaveNote(data, path)
	if(!IsEmpty(str))
	serStr += key + ":\r" + str + "\r"
	continue
	endif
	WAVE/Z wvn = JWN_GetNumericWaveFromWaveNote(data, path)
	if(WaveExists(wvn))
	str = NumericWaveToList(wvn, "\r")
	serStr += key + ":\r" + str
	continue
	endif
	WAVE/Z/T wt = JWN_GetTextWaveFromWaveNote(data, path)
	if(WaveExists(wt))
	str = TextWaveToList(wt, "\r")
	serStr += key + ":\r" + str
	continue
	endif
	endfor
	endif

[Copilot]

SFO_OperationIVSCCApFrequency constructs a SweepFormula string by interpolating experiment file names from uniqueFiles directly into expressions like selexp("%s") and into the ivscc_apfrequency_explist array without any escaping. On platforms where file names can contain " or other special characters, an attacker who controls experiment file names (e.g. via a crafted NWB/PXP on disk) can inject additional SweepFormula operations by breaking out of the quoted string (for example with a name like evil");otherOp()), which will then be executed under the user’s context. To avoid this injection vector, validate file/experiment names to a strict safe character set or escape them appropriately for the SweepFormula string literal context before inserting them into formula via sprintf/SF_AddExpressionToFormula.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.