Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Unused parameters in simulations #1050

Open
Yuri05 opened this issue Dec 18, 2018 · 8 comments
Open

RFC: Unused parameters in simulations #1050

Yuri05 opened this issue Dec 18, 2018 · 8 comments
Labels
RFC Request For Comments type: feature

Comments

@Yuri05
Copy link
Member

Yuri05 commented Dec 18, 2018

Currently, our Building Blocks (Individual first of all) contain parameters which can be used in all kind of models (small molecules vs. proteinmodel, different partition coeff. models etc.).

Once a simulation is created, many of those parameters are unused (e.g. protein model specific parameters in a small molecules simulation).

In the most cases it's not a problem (e.g. it does not slow down calculation of simulation results).
There are some use cases however where unused parameters have significant impact on performance:

  • e.g. computing sensitivity analysis, when selecting all non-formula parameters for variation.
  • e.g. when handling parameters in MoBi-Toolboxes for Matlab/R (searching for parameters in parameter tables etc.)

How should we deal with this?

  • Option 1: eliminate all unused parameters when creating a simulation. Will have some implications on update/commit from building block, showing differences between simulation and building block etc.
  • Option 2: keep those parameters in a simulation but mark them as unused. Then:
    • Set all unused parameters hidden, non-editable and CanBeVaried=false
    • Don't export those parameters to SimModel
  • Option 3: just set all unused parameters hidden, non-editable and CanBeVaried=false
  • Option 4: eliminated unused parameters in SimModel (will solve the problems on Toolbox-Level; however not in PK-Sim/MoBi)

@msevestre @KatrinCoboeken @PavelBal @StephanSchaller Thoughts on this?

@Yuri05 Yuri05 added the RFC Request For Comments label Dec 18, 2018
@StephanSchaller
Copy link
Member

I like option Numero Due (2).

With regards to #1041 , you would then have to add the property "non-editable = true" everywhere you check for "CanBeVaried=false" with an "OR".

@msevestre
Copy link
Member

msevestre commented Dec 18, 2018

e.g. when handling parameters in MoBi-Toolboxes for Matlab/R (searching for parameters in parameter tables etc.)

This should have absolutely no effect on performance using an appropriate data structure. Searching for parameters with a hash, even in list with over 10000 entries, should be instantaneous

In my opinion, removing unused parameters is a mistake. That's what we were doing before. It has its charm of course but until we manage to get a more modular structure, this will bite us.

So Option 1 is out.

I think also that not exporting parameters to SimModel might be a mistake, especially if we want to ensure we can swap species etc.. later in R or Matlab (@StephanSchaller I am looking at you!)

So Option 2 is out for me as well

I would go with Option 3. However I am not sure I would CHANGE the parameter flags that come from the DB (Visible, Editable etc..)

Maybe we need a transient flag: e.g. USED that cannot be set from the UI and is computed when creating the simulation

Finding unused parameters, marking them as such when creating a simulation (is it as easy as : the parameter does not appear in any RHS?). Then PI, SA etc.. could filter out those parameters. Matlab/R toolbox should also filter them out, or show them only based on flags etc..

So option 3~ for me :)

EDIT:

  • Finding unused parameters is a bit harder than that. As you can have P1 -> P2 ->P3 and P1 is not used anywhere. But then P2 and P3 would not be marked as UNUSED. So this needs to be done somehow recursively if we want to make it optimal

  • When we run a simulation from PKSim and MoBi, we export the xml using the Optimized flag. Maybe this could also ensure that UNUSED parameters are not exported to SimModel. For transient simulation runs, where the Xml is discarded immediately, this could be a slight memory/speed improvement?

@msevestre msevestre changed the title Unused parameters in simulations RFC: Unused parameters in simulations Dec 18, 2018
@msevestre msevestre pinned this issue Dec 18, 2018
@StephanSchaller
Copy link
Member

I think also that not exporting parameters to SimModel might be a mistake, especially if we want to ensure we can swap species etc.. later in R or Matlab (@StephanSchaller I am looking at you!)

I can feel it...*shudder* ;-)

… but I thought we also agreed to align parameter structure between species, so this would not be an issue. But maybe you are right, removing parameters from the SimModel may restrict us.

But of course this depends on what type of parameters would be hidden. Currently, the difference in parameter space for animal and human is not present in the respective other species (i.e. the parameters in animal, that are "missing" from human are not included in an animal model export)

I'll have to ponder that.

@Yuri05
Copy link
Member Author

Yuri05 commented Dec 18, 2018

This should have absolutely no effect on performance using an appropriate data structure. Searching for parameters with a hash, even in list with over 10000 entries, should be instantaneous

Well, toolboxes allow some operations on parameters like e.g. searching with wildcards (e.g. *Volume*), which makes hashing much harder.

I think also that not exporting parameters to SimModel might be a mistake, especially if we want to ensure we can swap species etc.. later in R or Matlab

I disagree. If there are no structural changes between species, then parameter not used for one species will also be unused for another one (thus species change in a Toolbox just via parameter values changes will still be possible)
If there are structural changes: SimModel-Export will not be sufficient for species change in any case. So it does not matter if unused parameters are exported or not as well.

Maybe we need a transient flag: e.g. USED that cannot be set from the UI and is computed when creating the simulation

Think this would be OK. Not so many implications compared to option 1.

Finding unused parameters is a bit harder than that. As you can have P1 -> P2 ->P3 and P1 is not used anywhere. But then P2 and P3 would not be marked as UNUSED. So this needs to be done somehow recursively if we want to make it optimal

Exactly.
P1 = all parameters used in: RHS, Initial Values, Observers, Event Conditions/Assignments etc.
P2 = all parameters used by P1 which are not yet in P1.
P3 = all parameters used by P2 which are not in P1 and not in P2
And so on until all used parameters are identified.

When we run a simulation from PKSim and MoBi, we export the xml using the Optimized flag. Maybe this could also ensure that UNUSED parameters are not exported to SimModel. For transient simulation runs, where the Xml is discarded immediately, this could be a slight memory/speed improvement?

I still would vote for removing unused parameters from SimModel-Export not only in optimized by also in "normal" mode.

@msevestre
Copy link
Member

Well, toolboxes allow some operations on parameters like e.g. searching with wildcards (e.g. Volume), which makes hashing much harder.

Sorry but even that still should have no impact on performance.

disagree. If there are no structural changes between species, then parameter not used for one species will also be unused for another one (thus species change in a Toolbox just via parameter values changes will still be possible)
If there are structural changes: SimModel-Export will not be sufficient for species change in any case. So it does not matter if unused parameters are exported or not as well.

Species change was just an example. I could see cases where a parameter is unused in one case and used in another (GallBladder emptying for example) or some switch or whatever. I cannot foresee anything that can be done and therefore I would not delete stuff...until clearly proven that it has an impact on performance.

Also deleting unused parameters would have the following side effect: If you create a parameter that is just here to do some analyses (similar to an observer), and this parameter is not used anywhere... well it would be removed. How would you deal with that? (and of course don't tell me you would add yet another flag to not delete this parameter. We have enough flags here already)

All in all, having a flag USED or UNUSED would be a good state to be able to filter easily some info without compromising the structure.

@Yuri05
Copy link
Member Author

Yuri05 commented Dec 19, 2018

(GallBladder emptying for example) or some switch

All parameters changed by events or used in events (conditions, assignments) should be marked as Used of course.

Also deleting unused parameters would have the following side effect: If you create a parameter that is just here to do some analyses (similar to an observer), and this parameter is not used anywhere... well it would be removed. How would you deal with that?

Agree. Could happen. I could argue that in this case user should mark parameter as plottable (which is not a new flag ;) ) or define an observer based on this parameter. Otherwise I don't see any clue in creating such a parameter.

I would suggest the following:

  • We introduce Used (or UsedInModel) flag for parameters in simulations as you suggested above
    a. in PK-Sim/MoBi
    b. in SimModel (flag will be exported to SimModel-XML)
    c. in Matlab/R-toolboxes (parameter tables will become a new column Used or UsedInModel)
  • In PK-Sim/MoBi all unused parameters will be identified during simulation creation
    • In a simulation, those parameters could be marked with a different color
    • Also we could introduce an option "Hide unused parameters in a simulation"
  • We identify all workflows, were unused parameters should be explicitly excluded for performance reasons:
    1. PI
    2. SA
    3. ...
  • When we export temporary SimModel-XML on the fly in PK-Sim/MoBi, all unused parameters will be automatically eliminated
  • When we export permanent SimModel-XML for later usage in Toolboxes etc.
    • All parameters will be exported
    • Unused parameters will get new Attribute used="0" (or usedInModel="0")
  • In SimModel new simulation option will be defined: IgnoreUnusedParameters
    • If set to true, all unused parameters will be eliminated from the system
    • Default value is true false (=current behaviour: keep all parameters)
  • When initializing a new simulation in Matlab/R-Toolbox:
    • InitSimulation will become the possibility to pass the value of IgnoreUnusedParameters to SimModel
    • Default behaviour here should be IgnoreUnusedParameters=true (to be discussed; however I would strongly vote for elimination of unused parameters here as a default setting. In a 0.0001% of all cases, where some unused parameters might be of interest user should include them explicitly)
  • In Matlab/R-Toolbox: parameter tables will become a new column Used or UsedInModel (and thus can be also filtered out for SA and similiar workflows)

@msevestre
Copy link
Member

Default value is false (=current behaviour: keep all parameters)

Why not set it to true by default all the time? I think having them in the exported Xml is potentially useful (and again here, I just don't know so assuming it could be useful) but I don't think they add anything during the simulation?

I like this flow.

@Yuri05
Copy link
Member Author

Yuri05 commented Dec 20, 2018

Why not set it to true by default all the time? I think having them in the exported Xml is potentially useful (and again here, I just don't know so assuming it could be useful) but I don't think they add anything during the simulation?

Agree. Changed the workflow description above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request For Comments type: feature
Projects
None yet
Development

No branches or pull requests

3 participants