Ion Channel Simulator by AurelienJaquier · Pull Request #536 · openbraininstitute/obi-one

AurelienJaquier · 2026-01-21T08:34:12Z

This is not complete yet.
This is missing IonChannelModelStimulusUnion and IonChannelModelRecordingUnion implementation. Also the entitysdk implementation is not done yet.

…into simulate-icm

james-isbister

Thanks @AurelienJaquier , looks good!

I'll think through some alternatives for how we could approach the conductances for each selected ion channel, and the recording of any parameter for the selected ion channels. And then we can discuss soon!

james-isbister · 2026-01-22T13:30:37Z

obi_one/scientific/tasks/ion_channel_model_simulation.py

+        group=BlockGroup.STIMULI_RECORDINGS,
+        group_order=0,
+    )
+    # can we have recording union depending on what model we choose?


This seems like an extension of the: ui_element: entity_property_dropdown

At the moment we have the example:

node_set: NodeSetType = Field( ui_element="entity_property_dropdown", entity_type=EntityType.CIRCUIT, property=CircuitPropertyType.NODE_SET, title="Node Set", description="Name of the node set to use.", min_length=1, )

Which specifies that the dropdown should display values returned by the mapped_circuit_properties endpoint - specifically the values in the list under the key CircuitPropertyType.NODE_SET

I think we could have a RecordVariable(Block) (or another name), which has a parameter recorded_variable with:

ui_element="entity_property_dropdown",
entity_type=EntityType.ION_CHANNEL_MODEL,
property=CircuitPropertyType.RECORDABLE_VARIABLES,

In the circuit simulation / extraction case there is only one circuit though so the dropdown only shows variables for one circuit

But in this case there will be multiple ion channels (selected somewhere in the configuration) - so we will have to think about how it is represented in the schema that the dropdown for recorded_variable should display values for all the ion channels (grouped by ion channel).

For now though, I think we could start by making a mapped_ion_channel_model_properties endpoint as we have for circuits, and we can go from there? what do you think?

Good idea. I have implemented a mapped_ion_channel_properties_endpoint in my latest commit.

Here are some things that we should keep in the back of our minds: each ion channel can have multiple variables (although most of the time they will only have one), and we might have ion channels models with common variables (e.g. if we have two calcium mixing mechanisms, we would have calcium concentration as an available recording for both).
Also, we should always have voltage recording available in case the user wants to do some current clamp experiment.

Also, is the dropdown already somewhere in the platform? I would like to see what it looks like in the UI if possible.

Nice!

Just to be sure, for the common variables, is there a single value that are we recording (that is shared between the two mechanisms) or two values which are later combined into a third value (which we also might record)?

Atm we have a simple dropdown in the PredefinedNeuronSet. But on top of that we could add some sort of grouping by ion channel

For the common variable, it would be a single value that is shared between the two mechanisms. The only case right now would be the calcium concentration that can be modified by several mechanisms.

I like the grouping by ion channel, because it would make it very explicit from what ion channel the variable is recorded from.

james-isbister · 2026-01-22T13:35:21Z

obi_one/scientific/tasks/ion_channel_model_simulation.py

+        }
+
+    class Initialize(Block):
+        ion_channel_models: list[IonChannelModelFromID] = Field(


One approach would be to have a block_dictionary i.e similar to what we have for Stimuli and NeuronSets in simulation.

So there would be an "Add Ion Channel Model" button under a heading say "Ion Channel Models"

When the user presses this button, the a new block is added to the dictionary with two parameters:

ion_channel_model_entity - clicking on this launches a selection - similar to what we have for the selection of the ion channel recordings in the current ion channel build

conductance

I have added the block dictionary.

We will have edge cases of ion channel models with no conductances. In which case, I think we could have the conductance field being greyed out. I assume this will be handled by the UI, and I don't have to care for it in obi-one, correct?

AurelienJaquier · 2026-02-03T14:32:18Z

What is lacking in this PR right now:

proper seclamp implementation (waiting on new SONATA definition)
entitycore/entitysdk config/campaign entities (waiting on generic config/campaign entities)
a way to properly have recording blocks depending on ion channel model selected

AurelienJaquier · 2026-02-06T12:35:43Z

I tried implementing the recording union. Would that do the trick @james-isbister or is this implementation still missing something?

…into simulate-icm

eleftherioszisis · 2026-03-04T08:31:15Z

obi_one/scientific/blocks/ion_channel_model.py

+                "conductance_name__isnull": False,
+            },
+        },
+    )


I wonder can't we use normal pydantic schemas? It is the first time I see this kind of schema declaration and frankly it's far from intuitive.

Is there is a better way to specify an entity filter? This was suggested by @bilalesi as a filter which can easily be applied in the frontend, but ideally I would like a way of specifying a filter which is usable both in obi-one through entitysdk and the frontend

the suggested filter is the same as entitycore, and it should align strictly with the terminology and naming conventions defined by entitycore, it's not something specific to the FE

in this case, we are specifying the filter (key: value) that should be used in the configuration-editor and it must be exactly what the ui element is expecting to use it and render the expected data and not the type so i don't see why using pydantic schema here, may be i'm missing smthg ?

for the properties used model_selector_entity_type and model_selector_property_filter, wouldn't be better to have one single dictionary defining the query shape as:

entity_query or model_query = { type: xxxxx, filters: { conductance_name__isnull: xxxx, yyyy: aaaa }

I followed your suggestion in my latest commit @bilalesi . Does it look ok to you?

thanks @AurelienJaquier , looks ok !

obi_one/scientific/tasks/generate_simulations/config/circuit.py

eleftherioszisis · 2026-03-04T20:56:07Z

obi_one/scientific/tasks/generate_simulations/task/task.py

+        if isinstance(circuit, Circuit):
            L.info("initialize.circuit is a Circuit instance.")
-            self._circuit = self.config.initialize.circuit
-            self._sonata_config["network"] = self.config.initialize.circuit.path
+            self._circuit = circuit
+            self._sonata_config["network"] = circuit.path

        elif isinstance(
-            self.config.initialize.circuit,
-            (CircuitFromID, MEModelFromID, MEModelWithSynapsesCircuitFromID),
+            circuit,
+            (
+                CircuitFromID,
+                MEModelFromID,
+                MEModelWithSynapsesCircuitFromID,
+                FakeCircuitFromIonChannelModels,
+            ),


I’ve noticed several conditional checks (e.g., configuration value checks, isinstance of types) that route execution into different flows. This approach makes the code difficult to follow and reason about, as it’s not clear what will be executed for any given task without tracing multiple branches.

A more maintainable design would define a dedicated task for each simulation type, with each task encapsulating all logic and responsibilities specific to that simulation type. This would improve readability, separation of concerns, and testability.

In its current form, the code is hard to maintain and will likely become increasingly difficult to extend or modify safely over time.

Yes, thank you for highlighting this - these types of comments are very useful to us so please keep them coming. We integrated a lot of your suggestions about separattion of concerns last year, which has improved the code base a lot. Sometimes there is just a lag given how many other things we're working on

I agree that we need to improve readability, separation of concerns and testability - and this if statement logic needs to be majorly simplified or indeed removed as you suggest.

I'm not yet totally convinced about separating into separate tasks for each config but I'm totally open to it - I just need to think a bit more about it.

I'd be very happy to discuss this further at some point once I've had time to think about it.

eleftherioszisis · 2026-03-05T16:09:04Z

obi_one/scientific/library/ion_channel_model_circuit.py

+        else:
+            circuit_config_path = dest_dir / "circuit_config.json"
+
+        return MEModelCircuit(name="single_cell", path=str(circuit_config_path))


Having staging logic inside config classes feels out of place. Staging is a task-level concern, while config classes should just hold and provide configuration data. Mixing the two blurs responsibilities and can make the code harder to maintain. It’s usually better to keep config focused on data, and handle staging or other task-specific logic elsewhere.

Good point.
I was trying to stay close to the logic used in the from_id classes.
Maybe we can keep this logic for now (here and in from_id classes), and think about how to disentangle staging logic from config classes in another PR after the webinar

Yeah, fine with me. I just left some comments here an there with things I noticed.

…into simulate-icm

AurelienJaquier · 2026-03-06T16:24:52Z

I think all the comments have been answered. Is this good to be merged or do you have more comments @james-isbister @eleftherioszisis @bilalesi ?

eleftherioszisis · 2026-03-06T16:56:37Z

Given the extent of the PR I cannot give my approval without any tests. Maybe someone that is more familiar with the correctness of the code or has tested the feature could approve.

….config.ion_channel_models

james-isbister

Very nice work, thank you.

I just applied some minor renaming:

config.ion_channel_model(s).py
Removed "Fake"

@AurelienJaquier Please give it a quick test with @bilalesi core-web-app branch locally. If it works as expected we can merge this and make a release

AurelienJaquier · 2026-03-09T08:04:37Z

How am I supposed to test it exactly?
I used the feat/146/ion-channel-ui-elements branch from core-web-app and followed your instructions to launch it, but then on the local version of the platform, I don't know how to test it: there is no button to launch an ion channel model simulation.

bilalesi · 2026-03-09T08:07:33Z

the simulation is not yet done, I will notify you when the branch preview link will be ready
feat/146/ion-channel-ui-elements this is for single neuron-beta and it's merged so you can test it in main.preview

…obi-one into simulate-icm

…into simulate-icm

AurelienJaquier · 2026-03-09T21:49:04Z

I tested with bilal's core web app branch and everything is good.
@james-isbister Should we merge this as it is, or should we proceed to add back the SEClamp stimulus and the new recordings?

JSONSchema GUI update

6b1a311

AurelienJaquier self-assigned this Jan 21, 2026

Merge branch 'main' of https://github.com/openbraininstitute/obi-one …

2bfaca9

…into simulate-icm

james-isbister reviewed Jan 21, 2026

View reviewed changes

james-isbister changed the title ~~JSONSchema GUI update~~ Ion Channel Simulator Jan 22, 2026

james-isbister reviewed Jan 22, 2026

View reviewed changes

AurelienJaquier added 7 commits January 23, 2026 11:41

add mapped ion channel properties endpoint

7bf5cfd

fixes

19b0461

Add block dictionary for ion channel models with conductances

e73f747

new ion channel block without conductance

a7ffe71

add has_conductance() method to IonChannelModelFromID

3d35d81

use new entitycore fields conductance_name and max_permeability_name

1d64057

add uncomplete template for seclamp stimulus

f7f9c97

AurelienJaquier added 3 commits February 3, 2026 15:33

small fix

00986f4

add get_other_parameter_names method

8eebcfe

add recording union and recording block

315a2b0

AurelienJaquier added 12 commits February 16, 2026 11:10

temporary seclamp implementation

a1c8993

improvements

8bbcd6e

transition to json_schema_extra

4c17b51

endpoint accepts multiple ids and returns variables and units

a15d9a2

fix stimulus

1eb78fa

improve seclamp

8bf5c47

Merge branch 'main' of https://github.com/openbraininstitute/obi-one …

5eb623a

…into simulate-icm

simplified SEClamp without timestamps

72005c9

runs locally

05810a7

refactoring and use generic campaign entities

b1decdd

Merge branch 'main' of https://github.com/openbraininstitute/obi-one …

dc9d674

…into simulate-icm

lint fix

35138cb

eleftherioszisis reviewed Mar 4, 2026

View reviewed changes

obi_one/scientific/tasks/generate_simulations/config/circuit.py Show resolved Hide resolved

james-isbister added 3 commits March 4, 2026 16:39

Merge branch 'main' into simulate-icm

1784b16

Move generate sim task

0de9b05

Use Simulation and SimulationCampaign entities

e014ccb

eleftherioszisis reviewed Mar 4, 2026

View reviewed changes

eleftherioszisis reviewed Mar 5, 2026

View reviewed changes

AurelienJaquier added 7 commits March 6, 2026 12:04

use entity_query and add back ion channel model block

9311d46

Specify radius of soma for ion channel staging

b75e952

remove generic entities

1846188

Merge branch 'main' of https://github.com/openbraininstitute/obi-one …

e5f81b9

…into simulate-icm

uv lock entitysdk

b3b185a

lint fix

2311867

add manipulations to memodel simulation group order

cef0175

james-isbister added 2 commits March 6, 2026 19:06

Remove Fake

b8242af

generate_simulations.config.ion_channel_model to generate_simulations…

b9f23f8

….config.ion_channel_models

james-isbister previously approved these changes Mar 6, 2026

View reviewed changes

Merge branch 'main' into simulate-icm

ee4dd87

AurelienJaquier added 3 commits March 9, 2026 13:09

fix circuit origin

926e255

Merge branch 'simulate-icm' of https://github.com/openbraininstitute/…

2f68a52

…obi-one into simulate-icm

fix id_str

fcd45ad

AurelienJaquier dismissed james-isbister’s stale review via fcd45ad March 9, 2026 12:23

AurelienJaquier added 3 commits March 9, 2026 22:38

use seclamp only for ion channel simulation

aa2e7da

Merge branch 'main' of https://github.com/openbraininstitute/obi-one …

a12311b

…into simulate-icm

lint fix

3252696

Conversation

AurelienJaquier commented Jan 21, 2026

Uh oh!

james-isbister left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AurelienJaquier commented Feb 3, 2026

Uh oh!

AurelienJaquier commented Feb 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

james-isbister Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AurelienJaquier commented Mar 6, 2026

Uh oh!

eleftherioszisis commented Mar 6, 2026

Uh oh!

james-isbister left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AurelienJaquier commented Mar 9, 2026

Uh oh!

bilalesi commented Mar 9, 2026

Uh oh!

AurelienJaquier commented Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

james-isbister Mar 4, 2026 •

edited

Loading

james-isbister left a comment •

edited

Loading