Open-Systems-Pharmacology / MoBi

MoBi® is a software tool for multiscale physiological modeling and simulation
Other
29 stars 9 forks source link

Updating simulation with changed events BB not working #298

Open AndreDlm opened 6 years ago

AndreDlm commented 6 years ago

In the attached simulation, I have changed some administration related parameter values in the events building block (specifically, the administered total dose and the dose per kg body weight). In the simulation, the red exclamation mark appears, but when I update the simulation through the context menu, the values are still the old ones, although the green tick mark is shown again.

Interestingly, if I change these values in the simulation, the red exclamation mark does not appear and MoBi does not recognize any difference between the simulation and event BB.

EventsBB.zip

PavelBal commented 6 years ago

The problem is - once you change events-related parameters in the simulation, the simulation "thinks" that these parameter are in the Parameters Start Values (PSV)-BB. E.g., if you would commit changes to PSV-BB, the entries for these parameters would be created.

I strongly vote for treating Events-Parameters in the same way as Reactions-Parameters, i.e., do not write changes into PSV-BB.

AndreDlm commented 6 years ago

Interesting. So I see the technical difficulty for the way back from an event-related change in the simulation to the event BB. Indeed, writing changes into the PSV BB might interfere with the desired change in other situations. However, the first part of this issue still remains: changing a reaction parameter value in the BB and updating it in the simulation works fine (i.e. the new value is propagated from the BB to the simulation), but this is not the case for the described change in the event BB. Instead, when you perform an update, the red exclamation mark for the event BB in the simulation is replaced by the green tick mark, but the values are not updated, which can be very misleading. I would strongly argue to fix that (or display any kind of warning message).

msevestre commented 6 years ago

However, the first part of this issue still remains: changing a reaction parameter value in the BB and updating it in the simulation works fine

This also depends on the parameter you are changing. If the parameter is GLOBAL (e.g. one instance), then it will work as you expect

if the parameter is LOCAL (e.g. one instance per reaction), the behavior will exactly be the same as the one you say with Event. The parameter value will be written in PSV.

It's very hard to be ALWAYS consistent in MoBi. Using PSV is a good as it gets until we come up with a better concept.

Instead, when you perform an update, the red exclamation mark for the event BB in the simulation is replaced by the green tick mark, but the values are not updated, which can be very misleading.

It may be misleading but this is the correct behavior. Basically by doing an update, you are using the event BB. But you still have values in your PSV that are overriding the event BB. In other words, parameter value in PSV always WIN. This is absolutely consistent throughout the usage of PSV

msevestre commented 6 years ago

@AndreDlm as a user, you are responsible to ensure that PSV is what you expect it to be. Remember, PSV always WIN

PavelBal commented 6 years ago

Using PSV is a good as it gets until we come up with a better concept.

I once proposed a concept of single PSV per each BB (Structures, Molecules, Reactions, Transports, Events). I still think that this concept would make life much more easier and the flexibility would GREATLY benefit (e.g., the user can combine different Structures-PSVs, which can describe different individuals, with different reactions/molecules PSV).

as a user, you are responsible to ensure that PSV is what you expect it to be. Remember, PSV always WIN

The problem is that you DO NOT expect Events-parameters to be in the PSV-BB! They are never there until you change such a parameter in the simulation and then commit the PSV-BB. The Events-parameter is then added to the PSV-BB, and the entry is "blue" - no origin found.

From my point of view, events-parameters definietly do not belong into the PSV!

msevestre commented 6 years ago

Even more PSV would make life easier? Here we are going to have to agree to disagree

I'd suggest we try to make it simple as now. The concept of PSV can be extended to all building blocks and does not have to be tied to spatial structure and molecule start value. Following the discussion we had in #118, we should make sure that the default PSV is empty! Then it would be clear that any parameter that the user wish to override without changing the building block can be found in the PSV. I am not sure how it could be any simpler than that.

The problem is that you DO NOT expect Events-parameters to be in the PSV-BB! They are never there until you change such a parameter in the simulation and then commit the PSV-BB. The Events-parameter is then added to the PSV-BB, and the entry is "blue" - no origin found. From my point of view, events-parameters definietly do not belong into the PSV!

None of that would be valid if we change the sole purpose of PSV. A simple list of parameters that are going to override building blocks parameter. A single flat searchable list. Not sure if it can get any easier than that

StephanSchaller commented 6 years ago

A simple list of parameters that are going to override building blocks parameter. A single flat searchable list. Not sure if it can get any easier than that

If this is what I think it is, it's a great Idea.

PavelBal commented 6 years ago

Following the discussion we had in #118, we should make sure that the default PSV is empty!

I already forgot that discussion :)

Two problems remain - relative expression, and parametrization of individuals. The idea of "clean PSV" would currently only work for a standard individual. As soon as new individual is imported (e.g., by calling "createPKSimIndividual" from Matlab, saving as xls, and loading in MoBi), the PSV again becomes overflooded with entries.

StephanSchaller commented 6 years ago

I think the outcome for this discussion also depends on future capabilities of MoBi. For MoBi with its current capabilities, I believe #118 is sth. we can agree on.

Capabilities that even exist now as mentioned by @PavelBal when loading individuals, or sth. envisioned for the future, like a plugin concept as drafted by @msevestre may indeed make a solution as mentioned by @PavelBal above:

… concept of single PSV per each BB ….

more feasible or even necessary. This plugin or "modularity" concept (loading/exchanging whole template structures like organs and PDmodels, individuals/populations) for MoBi would then also require extension of the "per each BB" rule to the MSV. Maybe sth best discussed in the plugin concept discussion.