small improvements of ion channel modeling form

[AurelienJaquier]

No description provided.

[AurelienJaquier]

As discussed today, I removed the entitysdk-dependent stuff from the PR so that we can merge it quickly. I will create another PR with the entitysdk registration code once entitysdk has all the needed functionnality.

[james-isbister]

• suffix: min_length=1 i.e.

  obi-one/obi_one/core/info.py

  Line 7 in 3cbac36

  campaign_name: str = Field(min_length=1, description="Name of the simulation campaign.")

• ion: Literal with options: https://github.com/openbraininstitute/obi-one/blob/3cbac362148faf0829206c4eaa330e20d9a9399b/obi_one/scientific/tasks/generate_simulation_configs.py#L151C26-L151C48
• temperature units (and other relevant variables):

  obi-one/obi_one/scientific/tasks/generate_simulation_configs.py

  Line 145 in 3cbac36

  units="mV",

• temperature; ge or gt

  obi-one/obi_one/scientific/tasks/generate_simulation_configs.py

  Line 109 in 3cbac36

  ge=_MIN_SIMULATION_LENGTH_MILLISECONDS, le=_MAX_SIMULATION_LENGTH_MILLISECONDS

• title="Gates parameters" -> "m & h gate exponents"?
• GatesParameters -> GateExponents
• act_stim_start, end + others -> NonNegativeFloat https://github.com/openbraininstitute/obi-one/blob/3cbac362148faf0829206c4eaa330e20d9a9399b/obi_one/scientific/tasks/generate_simulation_configs.py#L107C17-L107C33
• act_stim_start_correction -> float
• minf_eq: dict[str, equations_module.MInfUnion] -> minf_eq: equations_module.MInfUnion
• "Get ljp from metadata" explain briefly in execute
• No longer a dictionary:

  obi-one/obi_one/scientific/tasks/ion_channel_modeling.py

  Line 558 in 3f7f0cf

  "minf": next(iter(self.config.minf_eq.values())).equation_key,

[AurelienJaquier]

• suffix: min_length=1 i.e.

  obi-one/obi_one/core/info.py

  Line 7 in 3cbac36

  campaign_name: str = Field(min_length=1, description="Name of the simulation campaign.")

  • ion: Literal with options: https://github.com/openbraininstitute/obi-one/blob/3cbac362148faf0829206c4eaa330e20d9a9399b/obi_one/scientific/tasks/generate_simulation_configs.py#L151C26-L151C48

  • temperature units (and other relevant variables):

    obi-one/obi_one/scientific/tasks/generate_simulation_configs.py

    Line 145 in 3cbac36

    units="mV",

  • temperature; ge or gt

    obi-one/obi_one/scientific/tasks/generate_simulation_configs.py

    Line 109 in 3cbac36

    ge=_MIN_SIMULATION_LENGTH_MILLISECONDS, le=_MAX_SIMULATION_LENGTH_MILLISECONDS

  • title="Gates parameters" -> "m & h gate exponents"?

  • GatesParameters -> GateExponents

  • act_stim_start, end + others -> NonNegativeFloat https://github.com/openbraininstitute/obi-one/blob/3cbac362148faf0829206c4eaa330e20d9a9399b/obi_one/scientific/tasks/generate_simulation_configs.py#L107C17-L107C33

  • act_stim_start_correction -> float

  • minf_eq: dict[str, equations_module.MInfUnion] -> minf_eq: equations_module.MInfUnion

  • "Get ljp from metadata" explain briefly in execute

  • No longer a dictionary:

    obi-one/obi_one/scientific/tasks/ion_channel_modeling.py

    Line 558 in 3f7f0cf

    "minf": next(iter(self.config.minf_eq.values())).equation_key,

Done!

[james-isbister]

Looks good!

• Could the three variables with: default=-1 be changed to optional parameters with a default of None? This will be better for the UI I think

Also for this to be useable by core-web-app, we need to add it as an endpoint by adding the scan config here:

obi-one/app/endpoints/generated_endpoints.py

Line 96 in e35c2b8

(ContributeMorphologyScanConfig, "generate", ""),

We should then create an example notebook similar to:
https://github.com/openbraininstitute/obi-one/blob/main/examples/A_service_and_entitycore/circuit_simulation/entitysdk_circuit_simulations.ipynb

which:

1. Shows a working example from in Python
2. Shows a valid configuration which works when the service is launched using make run-local. Obviously this doesn't persist anything for now

[AurelienJaquier]

* Could the three variables with: `default=-1` be changed to optional parameters with a default of None? This will be better for the UI I think

The -1 here are not placeholders for None, they are actual values to be used. The fitting performs better when the end of stimulus is taken 1 ms before what is given by the file.

Also for this to be useable by core-web-app, we need to add it as an endpoint by adding the scan config here:

obi-one/app/endpoints/generated_endpoints.py

Line 96 in e35c2b8

(ContributeMorphologyScanConfig, "generate", ""),

We should then create an example notebook similar to: https://github.com/openbraininstitute/obi-one/blob/main/examples/A_service_and_entitycore/circuit_simulation/entitysdk_circuit_simulations.ipynb

which:

1. Shows a working example from in Python

2. Shows a valid configuration which works when the service is launched using `make run-local`. Obviously this doesn't persist anything for now

I added an example. However, it fails for now, since _single_entity is of type Any for now, since we do not have the entitysdk implementation yet.

[james-isbister]

Ok, then I think it would be good to include that -1 is good in explanation in the description of the 3 parameters.

And if the value is relative to the end of the file, then could they maybe be NonPositiveFloats which corresponds to le=0?

Once we have that I think we can merge, and fix the remaining issues when we have the entities?

[AurelienJaquier]

Ok, then I think it would be good to include that -1 is good in explanation in the description of the 3 parameters.

Done in latest commit

And if the value is relative to the end of the file, then could they maybe be NonPositiveFloats which corresponds to le=0?

While I expect people to use 0 or negative values here most of the time, there might be use cases where they have to use positive floats, so I would not like to restrain it more than necessary.

Once we have that I think we can merge, and fix the remaining issues when we have the entities?

Ok, good to be merged form my point of view!

[AurelienJaquier]

There was a bug in efel when the *_stim_end_correction value was too high. This is fixed now with this efel PR: openbraininstitute/eFEL#10

This PR is good to be merged imo