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

#59 used parameters #60

Merged
merged 6 commits into from
Feb 7, 2019
Merged

#59 used parameters #60

merged 6 commits into from
Feb 7, 2019

Conversation

Yuri05
Copy link
Member

@Yuri05 Yuri05 commented Feb 5, 2019

No description provided.

@Yuri05 Yuri05 requested a review from hdiedam February 5, 2019 18:09
@@ -29,6 +29,9 @@ class SimulationTask
static void SetValuesBelowAbsTolLevelToZero(double * values, int valuesSize, double absTol);

static void CheckForNegativeValues(Species ** odeVariables, int numberOfVariables, double absTol);

//mark all parameters which are used in any ODE varaible or observer as unused
static void MarkUsedParameters(Simulation * sim);
Copy link
Member Author

Choose a reason for hiding this comment

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

That's the main routine, which is called between Simulation.Load and Simulation.Finalize.
It sets UsedInSimulation flag for every simulation parameter

{
// if NOT usedAsValue it is contained in switch statement
if (!_useAsValue)
if (!_useAsValue && !alwaysAppend)
Copy link
Member Author

Choose a reason for hiding this comment

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

@hdiedam Not sure why it is used like this for the model CPP-Export.
Thus I've defined an optional parameter alwaysAppend, which is false by default (:arrow_right: CPP Export remains AS IS).
For use cases described in Open-Systems-Pharmacology/PK-Sim#1050 parameters must be always attached

@@ -15,13 +15,14 @@ using namespace std;

SimulationOptions::SimulationOptions(void)
{
_validateXMLWithSchema = true;
_validateXMLWithSchema = false; //perform schema validation only if explicitly set by user
Copy link
Member Author

Choose a reason for hiding this comment

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

this has nothing to do with used parameters.
Just changed the default from true (="validate XML with SimModel-Schema") to false. Just to increase the performance.
PK-Sim/MoBi always set it to false (assuming models generated there are always valid :)
Matlab/R-Toolbox still can set it to true if required

_showProgress = false;
_executionTimeLimit = 0.0; //no limit of simulation execution time
_stopOnWarnings = true; //treat warnings of ODE solver as errors
_autoReduceTolerances = true; //automatically reduce solver tolerances
_writeLogFile = true;
_checkForNegativeValues = true;
_identifyUsedParameters = false;
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if we need the flag or if used parameters should be ALWAYS estimated.
Currently there is a posiibility to skip this step (in this case, ALL parameters will be set as used)

TObjectList<Parameter> & parameters = sim->Parameters();
int idx;

if (!sim->Options().IdentifyUsedParameters())
Copy link
Member Author

Choose a reason for hiding this comment

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

s. comment regarding this option above.

@@ -222,6 +222,54 @@ void SimulationTask::CheckForNegativeValues(Species ** odeVariables, int numberO

}

void SimulationTask::MarkUsedParameters(Simulation * sim)
Copy link
Member Author

Choose a reason for hiding this comment

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

Main routine. MArks all parameters as used, which are used in:

  • ODE Variables (RHS or initial formula)
  • Observers
  • Event conditions
  • Event assignment formulas

@@ -448,9 +448,14 @@ void Species::SetODEIndex(int newIndex)
m_ODEIndex = newIndex;
}

void Species::AppendUsedParameters(std::set<int> & usedParameterIDs) {
void Species::AppendUsedParameters(std::set<int> & usedParameterIDs, bool includeInitialFormula)
Copy link
Member Author

Choose a reason for hiding this comment

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

@hdiedam Not sure why initial formula is skipped when getting used parameters for CPP-Export.
Thus I added optional parameter includeInitialFormula (false per default, so CPP-Export is not affected)

Copy link
Collaborator

Choose a reason for hiding this comment

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

The cppExport uses that function primarily to distinguish between "local" parameters (switched or somehow depending on y and t) and "global" parameters (depending only on free parameters) that can be precomputed. If they are depending only on some initial value y(t_0) and not on the state trajectory y(t) they still can be considered global.

@@ -178,14 +178,14 @@ void Switch::AppendUsedVariables(set<int> & usedVariblesIndices)
_formulaChangeVector[i]->AppendVariablesUsedInNewFormula(usedVariblesIndices, emptySet);
}

void Switch::AppendUsedParameters(std::set<int> & usedParameterIDs)
void Switch::AppendUsedParameters(std::set<int> & usedParameterIDs, bool alwaysAppendInFormulaChange)
Copy link
Member Author

Choose a reason for hiding this comment

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

s. comment in FormulaChange above

@@ -21,6 +21,14 @@
<FieldDef Name="ExecutionTimeLimit" DataType="Double" Description="Execution time limit for single simulation run (0= no limit)" DefaultValue="0"/>
<Values>0</Values>
</Variable>
<Variable Key="ValidateWithXMLSchema">
Copy link
Member Author

Choose a reason for hiding this comment

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

Configuration file for Matlab/R-Toolbox extend with possibilities to

  • enable/disable validation with SimModel-Schema
  • enable/disable identifying used parameters (if false: ALL parameters will be marked as used)

@@ -61,6 +61,9 @@ class SimModelComp : public DCI::Component
bool m_ProcessDataFirstRun;

void LoadConfigurationFromParameterTable();
bool GetBooleanValueFrom(DCI::ITableHandle & hTab, const std::string & columnName, bool defaultValue);
double GetDoubleValueFrom(DCI::ITableHandle & hTab, const std::string & columnName, double defaultValue);
std::string GetStringValueFrom(DCI::ITableHandle & hTab, const std::string & columnName, const std::string & defaultValue="");
Copy link
Member Author

Choose a reason for hiding this comment

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

just some refactoring

@@ -0,0 +1,225 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Member Author

Choose a reason for hiding this comment

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

Simulation used in tests, exported from TestExportUsedParameters01.mbp3

@@ -22,6 +22,8 @@ namespace SimModelNative
bool _keepXMLNodeAsString; //original xml is required only for saving the simulation to XML
bool _useFloatComparisonInUserOutputTimePoints; //if set to true, float comparison will be used
//for user output time points.Otherwise: double
bool _identifyUsedParameters; //if set to false: ALL parameters will be marked as used in ODE variables/observes
Copy link
Member

Choose a reason for hiding this comment

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

is there a reason why comments are not above the variable? Kind of unusual

Copy link
Member Author

Choose a reason for hiding this comment

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

No arguable reason :)

@@ -55,7 +55,7 @@ class FormulaChange :

//used variables are only added into the list if UseAsValue=false
void AppendVariablesUsedInNewFormula(std::set<int> & usedVariblesIndices, const std::set<int> & variblesIndicesUsedInSwitchAssignments);
void AppendUsedParameters(std::set<int> & usedParameterIDs);
void AppendUsedParameters(std::set<int> & usedParameterIDs, bool alwaysAppend);
Copy link
Member

Choose a reason for hiding this comment

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

don't understand what this parameter means alwaysAppend

Copy link
Member Author

Choose a reason for hiding this comment

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

Is explained here: #60 (review)

@@ -100,7 +100,7 @@ class Species :

void SetODEIndex(int newIndex);

virtual void AppendUsedParameters(std::set<int> & usedParameterIDs);
virtual void AppendUsedParameters(std::set<int> & usedParameterIDs, bool includeInitialFormula=false);
Copy link
Member

Choose a reason for hiding this comment

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

What does this parameter mean includeInitialFormula
Sometime this extra parameter has a default value (here false) sometimes not (see FormulaChange above). Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is explained here: #60 (review)
The reason is: Species.AppendUsedParameters is called "on the top level"
a) from the current use case and
b) from the Model-CPP-Export).
As I didn't want to change b), I defined this parameter as optional.

FormulaChange.AppendUsedParameters is called only by it's "parent" (Switch.AppendUsedParameters), thus here there is no need for optional paramter

@Yuri05
Copy link
Member Author

Yuri05 commented Feb 7, 2019

Merging to continue.
@hdiedam you can still review

@Yuri05 Yuri05 merged commit dd253e7 into Open-Systems-Pharmacology:master Feb 7, 2019
@Yuri05 Yuri05 deleted the #59_used_parameters branch February 15, 2019 08:47
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.

3 participants