Skip to content

Conversation

jdcourcol
Copy link
Contributor

@jdcourcol jdcourcol commented May 22, 2025

Initial proposal for the scientists to review and discuss.

The goal is to store the output of the calibration performed by bluecellulab on a memodel.

  • It contains only the database model. As simple as possible.

  • REST end point and test will be added once feedback is provided.

  • end points are available.

  • import of calibration result.

Copy link
Contributor

@AurelienJaquier AurelienJaquier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This model looks good as it is. It makes sense that it inherits from Entity.

We would also need to remove the fields holding_current, threshold_current, and validation_status from MEModel model. And creating MEModelCalibrationResult for each MEModel when importing from nexus to keep the holding_current and threshold_current in the database

Jean-Denis Courcol and others added 10 commits May 26, 2025 21:56
See #168 for the specification.
Briefly; a `BrainAtlas` is an named entity that has an asset `annotation.nrrd` attached to it.
It is associated with a particular `BrainRegionHierarchy`
In addition, `BrainAtlasRegion` gives metadata for all the regions within a `BrainAtlas`;
* their volume in the `annotation.nrrd` if they are a leaf, None otherwise.
* an attached asset with the .obj mesh
@jdcourcol jdcourcol force-pushed the calibration_result branch from 9d06faa to 1efe91c Compare May 26, 2025 20:08
Comment on lines +834 to +835
holding_current: Mapped[float]
threshold_current: Mapped[float]
Copy link
Contributor

@g-bar g-bar May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implication of this is that now a SingleNeuronSimulation should have a reference to both an MEModal and an MEModelCalibrationResult.

Not sure how it is for SingleNeuronSynaptome which also reference MEModel

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum, @AurelienJaquier should we have multiple calibration result per memodel or just one ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have just one.

Copy link
Contributor

@g-bar g-bar May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If users create an MEModel, and run a simulation without running validation, then the resulting simulations use different currents (the dummy values in bluenaas). That's a different model than then one with a calibration result.

Is it possible to re-run validation? Is Calibration result itself immutable? If i re-run the validation service do I update the values in the existing CalibrationResult?, is it guaranteed not to result in different values and thus not 'invalidate' simulations ran using the previous values?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Calibration result itself immutable?

Calibration result is immutable. However, we can replace it with a new calibration result. (recomputing the calibration)
Which means that it would be a different calibration result id. Therefore the simulation result would likely be different.

Provenance should track the memodel id + calibration result id used.

Wrt invalidation, we don't have that concept so far.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if a change in a field of calibration result means the model will behave differently, then it should be a different entity.
Therefore, we would replace the previous calibration result entity, with a new one, with a different id.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@g-bar , I am trying to add an memodel to calibrationresult relationship:
calibration_result = relationship( "MEModelCalibrationResult", uselist=False, foreign_keys="MEModelCalibrationResult.calibrated_entity_id", lazy="joined", )

However this thing is giving me the following error:

from return response_schema_class.model_validate(row) in "router_read_one":
"Error extracting attribute: InvalidRequestError: 'MEModel.calibration_result' is not available due to lazy='raise' [type=get_attribute_error, input_value=<app.db.model.MEModel object at 0x12cbaaa80>, input_type=MEModel]"

I think you worked on that , would you have any clue ?

Copy link
Contributor

@g-bar g-bar May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we just had to add a joinedload to the MEModel response query. (See my latest commit).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is calibration_result part of the immutability constraints of the MEModel based on the immutability proposal the is currently pending? i.e. will we need to create a new MEModel id if a new calibration result is created?

If the MEModel results differ when a calibration result is present, then it should be part of its immutability constraints. Otherwise, it will be quite difficult to track when a config (that points to entities like memodels for example), which is an input to an activity, has changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is calibration_result part of the immutability constraints of the MEModel based on the immutability proposal the is currently pending? i.e. will we need to create a new MEModel id if a new calibration result is created?

no it will be a new calibration result. With respect to the current proposal, the previous calibration result would be "deprecated". The ME Model would stay the same.
Provenance should log the usage of the MEModel and the CalibrationResult.

@jdcourcol jdcourcol merged commit 6856995 into main Jun 2, 2025
1 check passed
@jdcourcol jdcourcol deleted the calibration_result branch June 2, 2025 10:56
GianlucaFicarelli added a commit that referenced this pull request Jun 2, 2025
* origin/main:
  Wire in assets to the BrainAtlas and BrainAtlasRead (#207)
  me model calibration result proposal (#188)
  Remove legacy distribution metadata from assets during import (#206)
  sort the returned hierarchy alphabetically (#201)
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.

5 participants