Skip to content

New ion channel model simulation campaign entity#540

Closed
AurelienJaquier wants to merge 13 commits intomainfrom
icm-simulation
Closed

New ion channel model simulation campaign entity#540
AurelienJaquier wants to merge 13 commits intomainfrom
icm-simulation

Conversation

@AurelienJaquier
Copy link
Contributor

Ion channel model simulations will use (almost) the same workflow as Circuit and ME-Model simulations. So it will use the same Simulation and SimulationResults entities as well as SimulationExecution and SimulationGeneration activities.
But since it can have multiple ion channel models as input, opposed to the single circuit used as input in circuit simulations, I have to create this new IonChannelModelSimulationCampaign to refflect that.
I also make entity_id in Simulation optional, since we do not need it for ion channel model simulations (we already know the ion channel models used thanks to IonChannelModelSimulationCampaign).

@AurelienJaquier AurelienJaquier self-assigned this Feb 25, 2026
@AurelienJaquier
Copy link
Contributor Author

I am having a hard time fixing these tests. May I ask your help on this please @eleftherioszisis ?

@mgeplf
Copy link
Collaborator

mgeplf commented Feb 25, 2026

I am having a hard time fixing these tests. May I ask your help on this please @eleftherioszisis ?

Did you add an alembig migration?

Looking at the error, it seems that there is table information missing, I think.

2026-02-25T11:20:32.1399917Z ERROR tests/test_validation.py::test_update_one__fail_if_generated_ids_unauthorized - sqlalchemy.exc.InvalidRequestError: One or more mappers failed to initialize - can't proceed with initialization of other mappers. Triggering mapper: 'Mapper[IonChannelModelSimulationCampaign(ion_channel_model_simulation_campaign)]'. Original exception was: Could not determine join condition between parent/child tables on relationship IonChannelModelSimulationCampaign.simulations - there are no foreign keys linking these tables.  Ensure that referencing columns are associated with a ForeignKey or ForeignKeyConstraint, or specify a 'primaryjoin' expression.
2026-02-25T11:20:32.1400660Z ERROR tests/test_validation.py::test_update_one__fail_if_generated_ids_unauthorized - sqlalchemy.exc.ProgrammingError: (psycopg2.errors.UndefinedTable) relation "ion_channel_model__ion_channel_model_simulation_campaign" does not exist```

@AurelienJaquier
Copy link
Contributor Author

I have been trying to do a migration, but I get an error when I do. It says there is no table for simulation_campaign_base, but that was my intent so that we use the child classes (SimulationCampaign and IonChannelModelSimulationCampaign) instead of the parent class (SimulationCampaignBase).
I have been fighting with this but I can't see the solution

@eleftherioszisis
Copy link
Contributor

First, could you please revert the refactoring part and focus on the new feature code? Refactoring can come in a subsequent PR.

@AurelienJaquier
Copy link
Contributor Author

The issue is that I need the simulation_campaign_id in Simulation to be compatible with both SimulationCampaign and IonChannelModelSimulationCampaign. I could not find a way to achieve this without refactoring SimulationCampaign to be a child of SimulationCampaignBase.
Is there a way to do it without refactoring?

@GianlucaFicarelli
Copy link
Collaborator

GianlucaFicarelli commented Feb 25, 2026

The issue is that I need the simulation_campaign_id in Simulation to be compatible with both SimulationCampaign and IonChannelModelSimulationCampaign. I could not find a way to achieve this without refactoring SimulationCampaign to be a child of SimulationCampaignBase. Is there a way to do it without refactoring?

Instead of defining IonChannelModelSimulationCampaign and subclass SimulationCampaignBase, would the generic campaign approach work for that use case, as defined in https://github.com/openbraininstitute/prod-build-ion-channel-model/issues/85 ?
It can be simpler than subclassing and I can proceed on on that PR.

@AurelienJaquier
Copy link
Contributor Author

@GianlucaFicarelli Yes indeed, the generic campaign approach would solve this issue.
I opened this PR so that I am not blocked by the generic campaign. But if implementing the generic campaign is simpler and faster than this PR, then I can close my PR and use the generic campaign instead

@eleftherioszisis
Copy link
Contributor

@GianlucaFicarelli Yes indeed, the generic campaign approach would solve this issue. I opened this PR so that I am not blocked by the generic campaign. But if implementing the generic campaign is simpler and faster than this PR, then I can close my PR and use the generic campaign instead

imo it would be better to use the generic campaign because you would need to essentially follow similar approach but only for the specialized tables you are interested in. And the migration for doing that would be quite complex. @GianlucaFicarelli has already put a lot of work in doing that in his PR.

@AurelienJaquier
Copy link
Contributor Author

Alright, I'll be closing this then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants