new expandable_list ui element

[AurelienJaquier]

This is for this issue: https://github.com/openbraininstitute/prod-build-ion-channel-model/issues/77

For my use case, I need a list with an arbitrary number of (duration, voltage) pairs. I think it would be nice if this new ui_element is generalised to other use cases, but I am not sure how easy it would be to do.

I don't have any figma yet, so I did a quick drawing for the time being

[james-isbister]

Nice! Some initial thought

[james-isbister]

Note, a useful way of generating the schema is to add it to scientific/tasks/example_schema.py

Then it can easily be regenerated if it and when it needs to be edited

[james-isbister]

Yes, I think this is nice and makes sense

[james-isbister]

I'm not sure about the type here

As it's quite complex, we might want to create a class which holds the variables, with a function that returns whether or not we are scanning. We have started something similar here:

obi-one/obi_one/scientific/blocks/parameter_modifications/parameter_modifications.py

Line 27 in 678c760

class BySectionListModification(OBIBaseModel):

For example, you can imagine that we might have a new parent class ComplexVariableHolder with a function "return_multi_value_parameters" which returns the multi_values to be scanned over. This could be called on all subclasses of ComplexVariableHolder (when they are detected as a type) in the part of the ScanGenerationTask where we usually detect the parameters to be scanned over through checking for lists:

obi-one/obi_one/core/scan_generation.py

Line 49 in 8b5bfef

def multiple_value_parameters(self, *, display: bool = False) -> list[MultiValueScanParam]:

We can give this a bit more thought

[james-isbister]

Maybe a different name for the ui_element? tuple_parameter_sweep perhaps?

We could also use this or something similar for Neuron IDs potentially. Just an idea

Units will also be good to represent.

And maybe we can use something like subelement_titles and subelement_ui_elements.

We should also specify that only simply subelement_ui_elements can be used to begin with (probably float_parameter_sweep and int_parameter_sweep are enough for now)

[AurelienJaquier]

Units will also be good to represent.

Good point, I will add units

And maybe we can use something like subelement_titles and subelement_ui_elements.

Yes, that sounds better. I will use those

We should also specify that only simply subelement_ui_elements can be used to begin with (probably float_parameter_sweep and int_parameter_sweep are enough for now)

Sounds good, will do

We could also use this or something similar for Neuron IDs potentially.

Are you talking about MEModelFromID? In what context? Can you link to the class / code you are refering to?

Maybe a different name for the ui_element? tuple_parameter_sweep perhaps?

I don't think that tuple_parameter_sweep is a good name, because we can do the sweep on the subelements inside the tuple (here, the floats), but we cannot do a sweep on the tuple itself, that would be weird. Also we lose the notion that the list is "expandable", i.e. that it can have any number of tuples in it.

[codecov]

Codecov Report

❌ Patch coverage is 66.66667% with 15 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
..._one/core/block_subunit/complex_variable_holder.py	57.89%	8 Missing ⚠️
obi_one/core/block.py	20.00%	3 Missing and 1 partial ⚠️
obi_one/scientific/blocks/stimuli/stimulus.py	70.00%	3 Missing ⚠️

Flag	Coverage Δ
pytest	69.94% <66.66%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
obi_one/core/scan_generation.py	82.91% <100.00%> (+2.01%)	⬆️
obi_one/core/schema.py	100.00% <100.00%> (ø)
obi_one/scientific/unions/unions_stimuli.py	100.00% <ø> (ø)
obi_one/scientific/blocks/stimuli/stimulus.py	61.99% <70.00%> (+0.22%)	⬆️
obi_one/core/block.py	92.59% <20.00%> (-7.41%)	⬇️
..._one/core/block_subunit/complex_variable_holder.py	57.89% <57.89%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[AurelienJaquier]

Hi @g-bar ! I would like to ask this to new ui element to be implement in the front-end next ticket assignment meeting. Could you have a look and tell me if this looks good to you?

[james-isbister]

Not sure whether we want to support parameter sweeps in this PR or leave it to a later PR. Is the ComplexVariableHolder logic ready to be considered?

For one, the design doesn't show us how parameter sweeps will look.

In theory specifying voltage and duration as "float_parameter_sweeps" as you have done might allow us to re-use some logic for display and validation of voltages and durations within each step. For @g-bar to comment on this.

There is also something possibly quite general about specifying ui_elements within each DurationVoltageCombination. i.e. specifying multiple sets of parameters (per "step" in this case) where each parameter has it's own ui_element type and bounds.

Maybe we can keep an eye to generalisation in the future, but just implement what is needed for "voltage_duration_ui_element" for now

[g-bar]

I agree with James' comments, in general be weary of premature abstraction.

Also what does parameter sweep within a param even mean, it risks becoming a bit intractable for users. The param already results in a combination of values, so you would have combinations of combinations 😵‍💫.

I'd leave only floats for voltage and duration fields.

[james-isbister]

Just spoke with @AurelienJaquier to clarify

Maybe he could get some designs for the proposed parameter sweep functionality and add it here? So we can assess the proposal fully

[AurelienJaquier]

So I would like to be able to perform parameter sweeps inside DurationVoltageCombination, so that the users could reproduce experimental stimuli in one config instead of having to do 10 configs.
For example, one activation stimulus would have 4 levels (so 4 voltage & duration combinations), and would look like this:

But to have the full set of activation stimuli, we actually want to see the behavior of the cell at different voltages for the second level (while keeping the other levels the same). See image below.

(the durations are different from the 1st image, but you see the idea)

So this could be done with multiple configs, but it would be much simpler for the user to just use the parameter scan.

I aknowledge that this might be a little bit difficult for the unexperienced user. They still can use the stimulus without parameter scan if it is confusing to them. At some point, we should have the stimuli being plotted on the space on the right in the UI, which would make everything much clearer.
But this is out of the scope of this ticket.

For the ComplexVariableHolder logic, I expect ComplexVariableHolder to always be inside a list that should not be turned into a parameter scan, but to contain variables that can be used in parameter scan (or not). And I wrote the multiple_value_parameters() method accordingly. This is consistent with what other ComplexVariableHolder use cases we might have in the future, right @james-isbister ?
Anyway, if you guys think we should not generalize too soon, I can still move this logic down to VoltagerDurationCombination for the time being.

[g-bar]

Thanks for clarification, I think we should separate conceptually "parameterSweep" from this case. A parameterSweep as the name implies applies to a parameter not to a field within a parameter and when added it generates a full new simulation. In this case a new value merely adds a trace to the same simulation.

Also IMO it behooves us to reduce mental friction for users, in this case having separate cases of interfaces int | list[int] is a bit overkill. So I'd keep it as a list[int] only, since the single value case is contained in the list case, and make the ui simpler than what we have for paramSweep, (no switching modes, only list mode). Also to keep it mentally separate from a parameterSweep UI should it confuse users.

And I'd refrain from nesting ui_element in ither ui_elements, at least for the current usecase.

For the ComplexVariableHolder logic, I expect ComplexVariableHolder to always be inside a list that should not be turned into a parameter scan, but to contain variables that can be used in parameter scan (or not).

I'd keep it a single value not a list, the use of paramSweep is not apparent, over just adding values to the same sim. (Also the param sweep UI for this ui_element, would be a nightmare).

[AurelienJaquier]

Ah sorry, I was not clear. This example with Activation stimuli in our case would still produce mutliple configs, giving multiple stimulation, each with one single stimulus trace.

[g-bar]

I understand now, in that case I don't think the proposal makes much sense, ultimately you're sweeping over the full param, not the voltage duration fields.

So I'd do the following

• Simplify the proposed design to take less space to define multiple steps each with a single voltage and duration value.

Then we add parameter sweep over that. This would be consistent with the model we have and much simpler conceptually for users.

[AurelienJaquier]

I understand now, in that case I don't think the proposal makes much sense, ultimately you're sweeping over the full param, not the voltage duration fields.

So I'd do the following

* Simplify the proposed design to take less space to define multiple steps each with a single voltage and duration value.

Then we add parameter sweep over that. This would be consistent with the model we have and much simpler conceptually for users.

I am not sure I understand what you want for the design. It already defines mutliple steps each with a single voltage and duration. Do you just want it to take less space?
This is what we have in the PR currently:

I have asked Loris to add the + button for the parameter scan, so if we decide to keep the parameter scan, it would look like this:

[g-bar]

I mean the parameter scan to be added to the whole "Voltage Levels and durations" param not the "voltage" or "duration" fields.

But actually we don't need the parameter scan at all, scanning over the param would be akin to creating a new stimulus.
So we can keep the original design but remove all lists, keep only single values.

If the user wants another experiment of this kind (within the same config) it suffices for them to define a new block dictionary entry.

[AurelienJaquier]

I mean the parameter scan to be added to the whole "Voltage Levels and durations" param not the "voltage" or "duration" fields.

I feel that this would be even more untractable for the user, if we start having multiple Voltage Levels and durations, each with multiple levels.

But actually we don't need the parameter scan at all, scanning over the param would be akin to creating a new stimulus.
So we can keep the original design but remove all lists, keep only single values.

If the user wants another experiment of this kind (within the same config) it suffices for them to define a new block dictionary entry.

So the use would not be to create a new stimulus in the same config (we are not allowed to do that with SEClamp), it would be to create multiple configs with different values in the stimulus, like we have for the parameter scan in the other stimuli.

[james-isbister]

Yes, the problem is with NEURON. We are quite limited with what we can do with this type of stimulus without introducing weird behaviour. This proposal with parameter sweeps is to maximise flexibility that ion channel modellers would require given those constraints. I would prefer to do it with multiple blocks and timestamps but unfortunately we’re limited there

[AurelienJaquier]

@g-bar Have I managed to convince you with this implementation?
Or do you have more comments about this PR?

[AurelienJaquier]

Is this good enough that I can ask for an UI implementation in the next ticket assignment meeting @g-bar ?

[james-isbister]

Do we have a design for scanning over duration and/or voltage within a single DurationVoltageCombination?

[AurelienJaquier]

Do we have a design for scanning over duration and/or voltage within a single DurationVoltageCombination?

I have this design: https://www.figma.com/design/akGPTH0WwNFDfSWSs3qAnh/OBI---UI-UX-2025?node-id=9277-12607&t=w7zJ6XMnNnrijloP-0
with the + button on the right to add values to the parameter scan.
I then expect to new fields to appear in the same way it does for any other parameter scan. Or do we need a design for that also?

[james-isbister]

Otherwise, the schema looks good to me. On my side it's good to propose in the ticket meeting though, although ideally we would have the design for the parameter scans before then

I'll still need to look more at the details of scanning over variables within a ComplexVariableHolder though, and how this would extend to other use cases, but this is separate from the frontend implementation

[james-isbister]

I have this design
Cool, I'd just put the "+" symbols within circles to be consistent with the current UI.

Then you could update the png in this PR with that

[AurelienJaquier]

Cool, I'd just put the "+" symbols within circles to be consistent with the current UI.

@loris-olivier-obi says he wants to have a simple + design without circles around for all the parameter scans. That is why we don't have the circles here

[james-isbister]

I think we should stay consistent for now. We shouldn't change the design across workflows in one small unrelated PR that other people aren't aware of

[g-bar]

The schema should be good, we can use the same component / design for the parameter sweep fields.