diff --git a/VERSION.txt b/VERSION.txt index a5589f37e..908436027 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1 +1 @@ -1.20261014.0 +1.20261015.0 diff --git a/src/api/libopencor/sedinstance.h b/src/api/libopencor/sedinstance.h index 9bdef29ad..f5c6a52d6 100644 --- a/src/api/libopencor/sedinstance.h +++ b/src/api/libopencor/sedinstance.h @@ -85,7 +85,8 @@ class LIBOPENCOR_EXPORT SedInstance: public Logger * * Start running, in a background thread, all the tasks associated with this instance. The results of the tasks * are (re)allocated before this method returns, so they can be retrieved while the tasks are being run (e.g., to - * plot them progressively). The simulation settings used are those in effect when this method is called. + * plot them progressively). The simulation settings and model changes used are those in effect when this method is + * called. * * @return @c true if a new run was started, @c false if a run is already in progress. */ diff --git a/src/sed/sedchangeattribute.cpp b/src/sed/sedchangeattribute.cpp index ccfc6d447..b887e68be 100644 --- a/src/sed/sedchangeattribute.cpp +++ b/src/sed/sedchangeattribute.cpp @@ -87,12 +87,19 @@ void SedChangeAttribute::Impl::serialise(xmlNodePtr pNode) const xmlAddChild(pNode, node); } -void SedChangeAttribute::Impl::apply(const SedInstanceTaskPtr &pInstanceTask, - const libcellml::AnalyserModelPtr &pAnalyserModel) +void SedChangeAttribute::Impl::apply(SedInstanceTask::Impl *pInstanceTaskPimpl, + const libcellml::AnalyserModelPtr &pAnalyserModel) const { - auto addCannotChangeWarning = [this](const std::string &pVariableName, - const std::string &pComponentName, - const char *pVariableType) { + // Note: we report our warnings to the instance task rather than to ourselves. Indeed, we are owned by a model, + // which may be shared by several instance tasks, and our warnings would otherwise accumulate every time we + // get applied. + + auto addInstanceTaskWarning = [pInstanceTaskPimpl](const std::string &pWarning) { + pInstanceTaskPimpl->addIssue(Issue::Type::WARNING, pWarning, "Change attribute"); + }; + auto addCannotChangeWarning = [&addInstanceTaskWarning](const std::string &pVariableName, + const std::string &pComponentName, + const char *pVariableType) { std::string warning; warning.reserve(pVariableName.size() + pComponentName.size() + 120); // NOLINT @@ -105,17 +112,16 @@ void SedChangeAttribute::Impl::apply(const SedInstanceTaskPtr &pInstanceTask, warning += pComponentName; warning += "' cannot be changed. Only state variables and constants can be changed."; - addWarning(warning); + addInstanceTaskWarning(warning); }; - auto *instanceTaskPimpl {pInstanceTask->pimpl()}; auto changeName {name(mComponentName, mVariableName)}; - const auto stateCount = instanceTaskPimpl->stateCount(); - const auto constantCount = instanceTaskPimpl->constantCount(); - const auto computedConstantCount = instanceTaskPimpl->computedConstantCount(); - const auto algebraicVariableCount = instanceTaskPimpl->algebraicVariableCount(); + const auto stateCount = pInstanceTaskPimpl->stateCount(); + const auto constantCount = pInstanceTaskPimpl->constantCount(); + const auto computedConstantCount = pInstanceTaskPimpl->computedConstantCount(); + const auto algebraicVariableCount = pInstanceTaskPimpl->algebraicVariableCount(); - if (instanceTaskPimpl->voiName() == changeName) { + if (pInstanceTaskPimpl->voiName() == changeName) { auto voiVariable {pAnalyserModel->voi()->variable()}; auto voiComponent {owningComponent(voiVariable)}; @@ -127,8 +133,8 @@ void SedChangeAttribute::Impl::apply(const SedInstanceTaskPtr &pInstanceTask, auto isParameterSet {false}; for (size_t i {0}; i < stateCount; ++i) { - if (instanceTaskPimpl->stateName(i) == changeName) { - instanceTaskPimpl->mChanges.emplace_back(instanceTaskPimpl->mStates + i, toDouble(newValue())); // NOLINT + if (pInstanceTaskPimpl->stateName(i) == changeName) { + pInstanceTaskPimpl->mChanges.emplace_back(pInstanceTaskPimpl->mStates + i, toDouble(newValue())); // NOLINT isParameterSet = true; @@ -138,8 +144,8 @@ void SedChangeAttribute::Impl::apply(const SedInstanceTaskPtr &pInstanceTask, if (!isParameterSet) { for (size_t i {0}; i < constantCount; ++i) { - if (instanceTaskPimpl->constantName(i) == changeName) { - instanceTaskPimpl->mChanges.emplace_back(instanceTaskPimpl->mConstants + i, toDouble(newValue())); // NOLINT + if (pInstanceTaskPimpl->constantName(i) == changeName) { + pInstanceTaskPimpl->mChanges.emplace_back(pInstanceTaskPimpl->mConstants + i, toDouble(newValue())); // NOLINT isParameterSet = true; @@ -152,7 +158,7 @@ void SedChangeAttribute::Impl::apply(const SedInstanceTaskPtr &pInstanceTask, const auto &computedConstants = pAnalyserModel->computedConstants(); for (size_t i {0}; i < computedConstantCount; ++i) { - if (instanceTaskPimpl->computedConstantName(i) == changeName) { + if (pInstanceTaskPimpl->computedConstantName(i) == changeName) { auto computedConstantVariable {computedConstants[i]->variable()}; auto computedConstantComponent {owningComponent(computedConstantVariable)}; @@ -169,7 +175,7 @@ void SedChangeAttribute::Impl::apply(const SedInstanceTaskPtr &pInstanceTask, const auto &algebraicVariables = pAnalyserModel->algebraicVariables(); for (size_t i {0}; i < algebraicVariableCount; ++i) { - if (instanceTaskPimpl->algebraicVariableName(i) == changeName) { + if (pInstanceTaskPimpl->algebraicVariableName(i) == changeName) { auto algebraicVariable {algebraicVariables[i]->variable()}; auto algebraicComponent {owningComponent(algebraicVariable)}; @@ -193,7 +199,7 @@ void SedChangeAttribute::Impl::apply(const SedInstanceTaskPtr &pInstanceTask, warning += mComponentName; warning += "' could not be found and therefore could not be changed."; - addWarning(warning); + addInstanceTaskWarning(warning); } } diff --git a/src/sed/sedchangeattribute_p.h b/src/sed/sedchangeattribute_p.h index afa8e1ff6..6be5ea9eb 100644 --- a/src/sed/sedchangeattribute_p.h +++ b/src/sed/sedchangeattribute_p.h @@ -46,7 +46,7 @@ class SedChangeAttribute::Impl: public SedChange::Impl void serialise(xmlNodePtr pNode) const override; - void apply(const SedInstanceTaskPtr &pInstanceTask, const libcellml::AnalyserModelPtr &pAnalyserModel); + void apply(SedInstanceTask::Impl *pInstanceTaskPimpl, const libcellml::AnalyserModelPtr &pAnalyserModel) const; }; } // namespace libOpenCOR diff --git a/src/sed/sedinstance.cpp b/src/sed/sedinstance.cpp index bd00b2a01..4a4d23930 100644 --- a/src/sed/sedinstance.cpp +++ b/src/sed/sedinstance.cpp @@ -59,6 +59,8 @@ SedInstance::Impl::Impl(const SedDocumentPtr &pDocument) } // Create an instance of all the tasks, if they are all valid. + // Note: a task instance that has some issues (e.g., its model could not be compiled) cannot be run (see + // prepareRun()), hence its issues are reported and kept track of (see below). if (tasksValid) { mTasks.reserve(tasks.size()); @@ -68,9 +70,11 @@ SedInstance::Impl::Impl(const SedDocumentPtr &pDocument) mTasks.push_back(taskInstance); +#ifndef CODE_COVERAGE_ENABLED if (taskInstance->hasIssues()) { addIssues(taskInstance, "Task instance"); } +#endif } } } else { @@ -83,6 +87,31 @@ SedInstance::Impl::Impl(const SedDocumentPtr &pDocument) mTasksIssues = mIssues; mTasksErrors = mErrors; mTasksWarnings = mWarnings; + + // Initialise the task instances that have no issues, so that we can report the issues with their changes (e.g., a + // change that cannot be applied) and their initialisation (e.g., an NLA system that cannot be solved), and reset + // those issues so that they don't prevent those task instances from being run. + // Note: those issues are not kept track of since they depend on the changes to the model and on the simulation + // settings, both of which may be changed before the instance is run. Instead, the changes of a task instance + // are retrieved again and the task instance is initialised again every time it is run, at which point those + // issues get reported again, if still relevant (see prepareRun() and executeRun()). + + for (const auto &task : mTasks) { +#ifndef CODE_COVERAGE_ENABLED + if (!task->hasIssues()) { +#endif + auto *taskPimpl {task->pimpl()}; + + taskPimpl->retrieveChanges(); + taskPimpl->initialise(); + + addIssues(task, "Task instance"); + + taskPimpl->removeAllIssues(); +#ifndef CODE_COVERAGE_ENABLED + } +#endif + } } SedInstance::Status SedInstance::Impl::status() const @@ -140,9 +169,12 @@ void SedInstance::Impl::prepareRun() } // Prepare all the tasks associated with this instance to be run unless they have some issues. - // Note: a task may throw an exception (e.g., std::bad_alloc if its results cannot be allocated), in which case we - // report it as an error rather than let it escape and we don't run any of our tasks. We have no way to - // trigger such an exception in our tests, hence we ignore our try...catch statement during code coverage. + // Note #1: a task may throw an exception (e.g., std::bad_alloc if its results cannot be allocated), in which case + // we report it as an error rather than let it escape and we don't run any of our tasks. We have no way to + // trigger such an exception in our tests, hence we ignore our try...catch statement during code coverage. + // Note #2: a task only has issues at this stage if its instance could not be created (e.g., its model could not be + // compiled, see Impl()), which we have no way to trigger in our tests, hence we ignore our check during + // code coverage. mTasksToRun.clear(); @@ -152,18 +184,27 @@ void SedInstance::Impl::prepareRun() mTasksToRun.reserve(mTasks.size()); for (const auto &task : mTasks) { +#ifndef CODE_COVERAGE_ENABLED if (!task->hasIssues()) { - if (task->pimpl()->prepareRun()) { - mTasksToRun.push_back(task); - } else { - addIssues(task, "Task"); +#endif + const auto taskPrepared {task->pimpl()->prepareRun()}; - // Reset the issues of the task so that they are not reported again should the instance be run - // again. + // Report the issues of the task, if any (i.e. the errors about its simulation or the warnings about the + // changes that could not be applied to its model), and reset them so that they are not reported again + // should the instance be run again and so that warnings don't prevent the task from being run. + + if (task->hasIssues()) { + addIssues(task, "Task"); task->pimpl()->removeAllIssues(); } + + if (taskPrepared) { + mTasksToRun.push_back(task); + } +#ifndef CODE_COVERAGE_ENABLED } +#endif } #ifndef CODE_COVERAGE_ENABLED } catch (const std::exception &exception) { diff --git a/src/sed/sedinstancetask.cpp b/src/sed/sedinstancetask.cpp index cddcb86fc..57c258c04 100644 --- a/src/sed/sedinstancetask.cpp +++ b/src/sed/sedinstancetask.cpp @@ -56,21 +56,7 @@ static emscripten::val toFloat64Array(std::span data) SedInstanceTaskPtr SedInstanceTask::Impl::create(const SedAbstractTaskPtr &pTask) { - auto res {SedInstanceTaskPtr {new SedInstanceTask {pTask}}}; - - res->pimpl()->mOwner = res; - - // Initialise the instance task, but only if there are no issues with it. - -#ifndef CODE_COVERAGE_ENABLED - if (!res->hasIssues()) { -#endif - res->pimpl()->initialise(); -#ifndef CODE_COVERAGE_ENABLED - } -#endif - - return res; + return SedInstanceTaskPtr {new SedInstanceTask {pTask}}; } SedInstanceTask::Impl::Impl(const SedAbstractTaskPtr &pTask) @@ -363,9 +349,14 @@ void SedInstanceTask::Impl::nanFillResults(size_t pFromIndex) nanFillRowTails(mResults.algebraicVariables, mAlgebraicVariableCount); } -void SedInstanceTask::Impl::applyChanges() +void SedInstanceTask::Impl::retrieveChanges() { // Retrieve our changes (see SedChangeAttribute::Impl::apply()). + // Note: we are called from the thread that creates our instance or that runs us or starts running us (see + // SedInstance::Impl::Impl() and prepareRun()), i.e. before we actually get run. This means that our changes + // are those that were in effect when we were asked to be run, even if they get changed while we are being + // run. It also means that we don't access our model while we are being run, so our model can safely be + // changed while we are being run. mChanges.clear(); @@ -377,17 +368,18 @@ void SedInstanceTask::Impl::applyChanges() ASSERT_NE(changeAttribute, nullptr); - changeAttribute->pimpl()->apply(mOwner.lock(), mAnalyserModel); - - addIssues(changeAttribute, "Change attribute"); + changeAttribute->pimpl()->apply(this, mAnalyserModel); } +} - // Apply our changes, which means setting our changed variables and reinitialising the variables that are - // initialised using another variable since our generated code initialised them before we could apply our changes. - // We need to do this both before computing our computed constants (since they may depend on a changed variable or - // a variable that is initialised using another variable) and after (since our generated code initialises a - // variable that depends on a computed constant when computing our computed constants, which means that it may - // override a change to that variable). +void SedInstanceTask::Impl::applyChanges() +{ + // Apply our changes (see retrieveChanges()), which means setting our changed variables and reinitialising the + // variables that are initialised using another variable since our generated code initialised them before we could + // apply our changes. We need to do this both before computing our computed constants (since they may depend on a + // changed variable or a variable that is initialised using another variable) and after (since our generated code + // initialises a variable that depends on a computed constant when computing our computed constants, which means + // that it may override a change to that variable). auto isChangedVariable = [this](const double *pVariable) { return std::ranges::any_of(mChanges, [pVariable](const auto &pChange) { @@ -600,8 +592,8 @@ bool SedInstanceTask::Impl::prepareRun() // Note: we are called from the thread that runs us or that starts running us (see SedInstance::Impl::prepareRun()), // i.e. before we actually get run. This means that our results are (re)allocated before we get run, so that // they can be safely retrieved while we are being run (e.g., to plot them progressively). This also means - // that our simulation settings are those that were in effect when we were asked to be run, even if they get - // changed while we are being run. + // that our simulation settings and model changes are those that were in effect when we were asked to be + // run, even if they get changed while we are being run. // Reset our progress counters. @@ -638,6 +630,14 @@ bool SedInstanceTask::Impl::prepareRun() allocateResults(mDifferentialModel ? mNumberOfSteps + 1 : 1); + // Retrieve our changes. + // Note: this must be done after (re)allocating our results. Indeed, retrieving our changes may result in some + // warnings (e.g., a change that cannot be applied) and should the (re)allocation of our results then throw an + // exception, those warnings would never be reported nor reset (see SedInstance::Impl::prepareRun()), which + // would prevent us from ever being run again. + + retrieveChanges(); + // Set our total number of steps. mTotalSteps.store(mNumberOfSteps, std::memory_order_relaxed); @@ -654,7 +654,8 @@ double SedInstanceTask::Impl::run() auto startTime {std::chrono::high_resolution_clock::now()}; // (Re)initialise our model. - // Note: reinitialise our model because we initialised it when we created the instance task. + // Note: reinitialise our model because it was initialised when our instance was created (see + // SedInstance::Impl::Impl()). initialise(); diff --git a/src/sed/sedinstancetask_p.h b/src/sed/sedinstancetask_p.h index 82b3c5a96..15ac274dc 100644 --- a/src/sed/sedinstancetask_p.h +++ b/src/sed/sedinstancetask_p.h @@ -64,12 +64,9 @@ struct SedInstanceTaskInitialisation using SedInstanceTaskInitialisations = std::vector; using SedInstanceTaskChanges = std::vector>; -using SedInstanceTaskWeakPtr = std::weak_ptr; - class SedInstanceTask::Impl: public Logger::Impl { public: - SedInstanceTaskWeakPtr mOwner; CellmlFileRuntimePtr mRuntime; SedSimulationPtr mSimulation; SedUniformTimeCoursePtr mSedUniformTimeCourse; @@ -136,6 +133,7 @@ class SedInstanceTask::Impl: public Logger::Impl void trackResults(size_t pIndex); void nanFillResults(size_t pFromIndex); + void retrieveChanges(); void applyChanges(); void initialise(); bool prepareRun(); diff --git a/tests/api/sed/coveragetests.cpp b/tests/api/sed/coveragetests.cpp index c3ed23570..3fcf15179 100644 --- a/tests/api/sed/coveragetests.cpp +++ b/tests/api/sed/coveragetests.cpp @@ -556,9 +556,12 @@ TEST(CoverageSedTest, sedUniformTimeCourse) TEST(CoverageSedTest, sedInstanceAndSedInstanceTaskDifferentialModel) { - static const libOpenCOR::ExpectedIssues EXPECTED_ISSUES {{ + static const libOpenCOR::ExpectedIssues EXPECTED_INSTANTIATION_ISSUES {{ {libOpenCOR::Issue::Type::ERROR, "Task instance | CVODE: the upper half-bandwidth cannot be equal to -1. It must be between 0 and 3."}, }}; + static const libOpenCOR::ExpectedIssues EXPECTED_RUN_ISSUES {{ + {libOpenCOR::Issue::Type::ERROR, "Task | CVODE: the upper half-bandwidth cannot be equal to -1. It must be between 0 and 3."}, + }}; static const auto UPPER_HALF_BANDWIDTH {-1}; auto file {libOpenCOR::File::create(libOpenCOR::resourcePath("api/solver/ode.cellml"))}; @@ -571,6 +574,8 @@ TEST(CoverageSedTest, sedInstanceAndSedInstanceTaskDifferentialModel) auto instance {document->instantiate()}; const auto &instanceTask {instance->tasks()[0]}; + EXPECT_EQ_ISSUES(instance, EXPECTED_INSTANTIATION_ISSUES); + EXPECT_EQ(instance->hasTasks(), true); EXPECT_EQ(instance->taskCount(), 1U); EXPECT_EQ(instance->task(0), instanceTask); @@ -622,7 +627,7 @@ TEST(CoverageSedTest, sedInstanceAndSedInstanceTaskDifferentialModel) instance->run(); - EXPECT_EQ_ISSUES(instance, EXPECTED_ISSUES); + EXPECT_EQ_ISSUES(instance, EXPECTED_RUN_ISSUES); } TEST(CoverageSedTest, sedInstanceAndSedInstanceTaskNonDifferentialModel) diff --git a/tests/api/sed/instancetests.cpp b/tests/api/sed/instancetests.cpp index 3a56276aa..966dc25f9 100644 --- a/tests/api/sed/instancetests.cpp +++ b/tests/api/sed/instancetests.cpp @@ -23,6 +23,7 @@ limitations under the License. #include #include #include +#include #include TEST(InstanceSedTest, noFile) @@ -847,6 +848,190 @@ TEST(InstanceSedTest, simulationSettingsUsedWhenStartingRun) EXPECT_EQ(instanceTask->voi()[instanceTask->voi().size() - 1], FACTOR * outputEndTime); } +TEST(InstanceSedTest, modelChangesUsedWhenStartingRun) +{ + // Note: the model changes used by a run are those in effect when the run is started, even if they get changed while + // the run is in progress. + + auto file {libOpenCOR::File::create(libOpenCOR::resourcePath("api/sed/variables_initialised_using_variables.cellml"))}; + auto document {libOpenCOR::SedDocument::create(file)}; + const auto &model {document->model(0)}; + auto change {libOpenCOR::SedChangeAttribute::create("main", "k", "5.0")}; + + model->addChange(change); + + auto instance {document->instantiate()}; + + EXPECT_TRUE(instance->startRun()); + + change->setNewValue("7.0"); + model->addChange(libOpenCOR::SedChangeAttribute::create("main", "x", "9.0")); + + EXPECT_GT(instance->waitForRun(), 0.0); + EXPECT_FALSE(instance->hasIssues()); + + const auto &instanceTask {instance->tasks()[0]}; + + EXPECT_EQ(instanceTask->stateName(0), "main/x"); + EXPECT_EQ(instanceTask->state(0)[0], 5.0); + + // Running our instance again should use our new model changes. + + EXPECT_GT(instance->run(), 0.0); + EXPECT_FALSE(instance->hasIssues()); + + EXPECT_EQ(instanceTask->state(0)[0], 9.0); + + model->removeChange(model->changes()[1]); + + EXPECT_GT(instance->run(), 0.0); + EXPECT_FALSE(instance->hasIssues()); + + EXPECT_EQ(instanceTask->state(0)[0], 7.0); +} + +TEST(InstanceSedTest, changeModelAfterStopRun) +{ + // Note: stopping a run doesn't wait for it to be done, so our model may get changed while the stopped run is still + // in progress, which must not affect it. + + static const auto NUMBER_OF_RUNS {1000}; + + auto file {libOpenCOR::File::create(libOpenCOR::resourcePath("api/sed/variables_initialised_using_variables.cellml"))}; + auto document {libOpenCOR::SedDocument::create(file)}; + const auto &model {document->model(0)}; + auto instance {document->instantiate()}; + + for (auto i {0}; i < NUMBER_OF_RUNS; ++i) { + ASSERT_TRUE(instance->startRun()); + + instance->stopRun(); + + model->removeAllChanges(); + model->addChange(libOpenCOR::SedChangeAttribute::create("main", "k", std::to_string(i))); + + instance->waitForRun(); + + ASSERT_FALSE(instance->hasIssues()); + } + + // Running our instance again should use our last model change. + + EXPECT_GT(instance->run(), 0.0); + EXPECT_FALSE(instance->hasIssues()); + + const auto &instanceTask {instance->tasks()[0]}; + + EXPECT_EQ(instanceTask->stateName(0), "main/x"); + EXPECT_EQ(instanceTask->state(0)[0], NUMBER_OF_RUNS - 1); +} + +TEST(InstanceSedTest, modelChangesThatCannotBeApplied) +{ + // Note: model changes that cannot be applied are reported and ignored, but they don't prevent an instance from + // being run, whether they are present when the instance is created or added afterwards. + + static const libOpenCOR::ExpectedIssues EXPECTED_INSTANTIATION_ISSUES {{ + {libOpenCOR::Issue::Type::WARNING, "Task instance | Change attribute: the variable 'unknown' in component 'main' could not be found and therefore could not be changed."}, + }}; + static const libOpenCOR::ExpectedIssues EXPECTED_RUN_ISSUES {{ + {libOpenCOR::Issue::Type::WARNING, "Task | Change attribute: the variable 'unknown' in component 'main' could not be found and therefore could not be changed."}, + }}; + static const libOpenCOR::ExpectedIssues EXPECTED_OTHER_RUN_ISSUES {{ + {libOpenCOR::Issue::Type::WARNING, "Task | Change attribute: the computed constant 'cc' in component 'main' cannot be changed. Only state variables and constants can be changed."}, + }}; + + auto file {libOpenCOR::File::create(libOpenCOR::resourcePath("api/sed/variables_initialised_using_variables.cellml"))}; + auto document {libOpenCOR::SedDocument::create(file)}; + const auto &model {document->model(0)}; + + model->addChange(libOpenCOR::SedChangeAttribute::create("main", "k", "5.0")); + model->addChange(libOpenCOR::SedChangeAttribute::create("main", "unknown", "7.0")); + + auto instance {document->instantiate()}; + + EXPECT_EQ_ISSUES(instance, EXPECTED_INSTANTIATION_ISSUES); + + EXPECT_GT(instance->run(), 0.0); + EXPECT_EQ_ISSUES(instance, EXPECTED_RUN_ISSUES); + + const auto &instanceTask {instance->tasks()[0]}; + + EXPECT_EQ(instanceTask->stateName(0), "main/x"); + EXPECT_EQ(instanceTask->state(0)[0], 5.0); + + // Removing our model change that cannot be applied and running our instance again should not report any issues. + + model->removeChange(model->changes()[1]); + + EXPECT_GT(instance->run(), 0.0); + EXPECT_FALSE(instance->hasIssues()); + + EXPECT_EQ(instanceTask->state(0)[0], 5.0); + + // Adding another model change that cannot be applied and running our instance again should report it. + + model->addChange(libOpenCOR::SedChangeAttribute::create("main", "cc", "9.0")); + + EXPECT_GT(instance->run(), 0.0); + EXPECT_EQ_ISSUES(instance, EXPECTED_OTHER_RUN_ISSUES); + + EXPECT_EQ(instanceTask->state(0)[0], 5.0); + + // Removing that model change and running our instance again should not report any issues. + + model->removeChange(model->changes()[1]); + + EXPECT_GT(instance->run(), 0.0); + EXPECT_FALSE(instance->hasIssues()); + + EXPECT_EQ(instanceTask->state(0)[0], 5.0); +} + +TEST(InstanceSedTest, initialisationFixedByModelChanges) +{ + // Note: an initialisation issue (e.g., an NLA system that cannot be solved) doesn't prevent an instance from being + // run once it has been fixed (e.g., using some model changes), even if it was present when the instance was + // created. + + static const libOpenCOR::ExpectedIssues EXPECTED_INSTANTIATION_ISSUES {{ + {libOpenCOR::Issue::Type::ERROR, "Task instance | KINSOL: the NLA system could not be solved (it contains some Inf and/or NaN values)."}, + }}; + static const libOpenCOR::ExpectedIssues EXPECTED_RUN_ISSUES {{ + {libOpenCOR::Issue::Type::ERROR, "Task | KINSOL: the NLA system could not be solved (it contains some Inf and/or NaN values)."}, + }}; + + auto file {libOpenCOR::File::create(libOpenCOR::resourcePath("api/sed/kinsol_with_inf_and_or_nan_values.cellml"))}; + auto document {libOpenCOR::SedDocument::create(file)}; + const auto &model {document->model(0)}; + auto instance {document->instantiate()}; + + EXPECT_EQ_ISSUES(instance, EXPECTED_INSTANTIATION_ISSUES); + + EXPECT_EQ(instance->run(), 0.0); + EXPECT_EQ_ISSUES(instance, EXPECTED_RUN_ISSUES); + + // Adding some model changes that fix our NLA system and running our instance again should not report any issues. + + model->addChange(libOpenCOR::SedChangeAttribute::create("main", "q_3", "1.0")); + model->addChange(libOpenCOR::SedChangeAttribute::create("main", "q_4", "1.0")); + + EXPECT_GT(instance->run(), 0.0); + EXPECT_FALSE(instance->hasIssues()); + + const auto &instanceTask {instance->tasks()[0]}; + + EXPECT_EQ(instanceTask->stateName(2), "main/q_3"); + EXPECT_EQ(instanceTask->state(2)[0], 1.0); + + // Removing those model changes and running our instance again should report our initialisation issue again. + + model->removeAllChanges(); + + EXPECT_EQ(instance->run(), 0.0); + EXPECT_EQ_ISSUES(instance, EXPECTED_RUN_ISSUES); +} + TEST(InstanceSedTest, runWhileAsynchronousRunInProgress) { // Note: running an instance while it is already being run asynchronously should wait for the asynchronous run to @@ -1245,9 +1430,12 @@ TEST(InstanceSedTest, nlaModelWithNoNlaSolver) TEST(InstanceSedTest, daeModel) { - static const libOpenCOR::ExpectedIssues EXPECTED_ISSUES {{ + static const libOpenCOR::ExpectedIssues EXPECTED_INSTANTIATION_ISSUES {{ {libOpenCOR::Issue::Type::ERROR, "Task instance | KINSOL: the upper half-bandwidth cannot be equal to -1. It must be between 0 and 0."}, }}; + static const libOpenCOR::ExpectedIssues EXPECTED_RUN_ISSUES {{ + {libOpenCOR::Issue::Type::ERROR, "Task | KINSOL: the upper half-bandwidth cannot be equal to -1. It must be between 0 and 0."}, + }}; auto file {libOpenCOR::File::create(libOpenCOR::resourcePath("api/sed/dae.cellml"))}; auto document {libOpenCOR::SedDocument::create(file)}; @@ -1259,11 +1447,11 @@ TEST(InstanceSedTest, daeModel) auto instance {document->instantiate()}; - EXPECT_EQ_ISSUES(instance, EXPECTED_ISSUES); + EXPECT_EQ_ISSUES(instance, EXPECTED_INSTANTIATION_ISSUES); instance->run(); - EXPECT_EQ_ISSUES(instance, EXPECTED_ISSUES); + EXPECT_EQ_ISSUES(instance, EXPECTED_RUN_ISSUES); kinsol->setLinearSolver(libOpenCOR::SolverKinsol::LinearSolver::DENSE); diff --git a/tests/api/solver/coveragetests.cpp b/tests/api/solver/coveragetests.cpp index b4d786203..f8f1e46a3 100644 --- a/tests/api/solver/coveragetests.cpp +++ b/tests/api/solver/coveragetests.cpp @@ -25,20 +25,50 @@ limitations under the License. TEST(CoverageSolverTest, odeChanges) { - static const libOpenCOR::ExpectedIssues EXPECTED_ISSUES {{ + static const libOpenCOR::ExpectedIssues EXPECTED_INSTANTIATION_ISSUES {{ {libOpenCOR::Issue::Type::WARNING, "Task instance | Change attribute: the variable of integration 'time' in component 'environment' cannot be changed. Only state variables and constants can be changed."}, {libOpenCOR::Issue::Type::WARNING, "Task instance | Change attribute: the variable 'X' in component 'membrane' could not be found and therefore could not be changed."}, {libOpenCOR::Issue::Type::WARNING, "Task instance | Change attribute: the computed constant 'E_Na' in component 'sodium_channel' cannot be changed. Only state variables and constants can be changed."}, {libOpenCOR::Issue::Type::WARNING, "Task instance | Change attribute: the algebraic variable 'i_Stim' in component 'membrane' cannot be changed. Only state variables and constants can be changed."}, }}; + static const libOpenCOR::ExpectedIssues EXPECTED_RUN_ISSUES {{ + {libOpenCOR::Issue::Type::WARNING, "Task | Change attribute: the variable of integration 'time' in component 'environment' cannot be changed. Only state variables and constants can be changed."}, + {libOpenCOR::Issue::Type::WARNING, "Task | Change attribute: the variable 'X' in component 'membrane' could not be found and therefore could not be changed."}, + {libOpenCOR::Issue::Type::WARNING, "Task | Change attribute: the computed constant 'E_Na' in component 'sodium_channel' cannot be changed. Only state variables and constants can be changed."}, + {libOpenCOR::Issue::Type::WARNING, "Task | Change attribute: the algebraic variable 'i_Stim' in component 'membrane' cannot be changed. Only state variables and constants can be changed."}, + }}; auto file {libOpenCOR::File::create(libOpenCOR::resourcePath("api/solver/ode_sed_changes.omex"))}; auto document {libOpenCOR::SedDocument::create(file)}; auto instance {document->instantiate()}; - instance->run(); + EXPECT_EQ_ISSUES(instance, EXPECTED_INSTANTIATION_ISSUES); + + // Running our instance should report the same issues, but our changes that could not be applied should have been + // ignored rather than prevent our instance from being run, i.e. our change that could be applied should have been + // applied. + + EXPECT_GT(instance->run(), 0.0); + EXPECT_EQ_ISSUES(instance, EXPECTED_RUN_ISSUES); + + const auto &instanceTask {instance->tasks()[0]}; + + EXPECT_EQ(instanceTask->stateName(0), "membrane/V"); + EXPECT_EQ(instanceTask->state(0)[0], 3.0); + + // Running our instance again should report the same issues (rather than accumulate them). + + EXPECT_GT(instance->run(), 0.0); + EXPECT_EQ_ISSUES(instance, EXPECTED_RUN_ISSUES); + + // Instantiating our document again should report the same issues (rather than accumulate them). + + auto otherInstance {document->instantiate()}; + + EXPECT_EQ_ISSUES(otherInstance, EXPECTED_INSTANTIATION_ISSUES); - EXPECT_EQ_ISSUES(instance, EXPECTED_ISSUES); + EXPECT_GT(otherInstance->run(), 0.0); + EXPECT_EQ_ISSUES(otherInstance, EXPECTED_RUN_ISSUES); } TEST(CoverageSolverTest, algebraicChanges) diff --git a/tests/bindings/javascript/sed.coverage.test.js b/tests/bindings/javascript/sed.coverage.test.js index 523b697db..43ef59945 100644 --- a/tests/bindings/javascript/sed.coverage.test.js +++ b/tests/bindings/javascript/sed.coverage.test.js @@ -661,6 +661,13 @@ test.describe('Sed coverage tests', () => { const instance = document.instantiate(); const instanceTask = instance.tasks[0]; + assertIssues(loc, instance, [ + [ + loc.Issue.Type.ERROR, + 'Task instance | CVODE: the upper half-bandwidth cannot be equal to -1. It must be between 0 and 3.' + ] + ]); + assert.strictEqual(instance.hasTasks, true); assert.strictEqual(instance.taskCount, 1); assert.deepStrictEqual(instance.task(0), instanceTask); @@ -715,7 +722,7 @@ test.describe('Sed coverage tests', () => { assertIssues(loc, instance, [ [ loc.Issue.Type.ERROR, - 'Task instance | CVODE: the upper half-bandwidth cannot be equal to -1. It must be between 0 and 3.' + 'Task | CVODE: the upper half-bandwidth cannot be equal to -1. It must be between 0 and 3.' ] ]); }); diff --git a/tests/bindings/javascript/sed.instance.test.js b/tests/bindings/javascript/sed.instance.test.js index e0303a349..fb46e81b6 100644 --- a/tests/bindings/javascript/sed.instance.test.js +++ b/tests/bindings/javascript/sed.instance.test.js @@ -919,6 +919,217 @@ test.describe('Sed instance tests', () => { assert.strictEqual(instanceTask.voi[instanceTask.voi.length - 1], 2.0 * outputEndTime); }); + test('Model changes used when starting run', () => { + // Note: the model changes used by a run are those in effect when the run is started, even if they get changed + // while the run is in progress. + + const file = new loc.File(utils.resourcePath('api/sed/variables_initialised_using_variables.cellml')); + + file.setContents(utils.fileContents(file.path)); + + const document = new loc.SedDocument(file); + const model = document.model(0); + const change = new loc.SedChangeAttribute('main', 'k', '5.0'); + + model.addChange(change); + + const instance = document.instantiate(); + + assert.strictEqual(instance.startRun(), true); + + change.newValue = '7.0'; + model.addChange(new loc.SedChangeAttribute('main', 'x', '9.0')); + + assert.ok(instance.waitForRun() > 0.0); + assert.strictEqual(instance.hasIssues, false); + + const instanceTask = instance.tasks[0]; + + assert.strictEqual(instanceTask.stateName(0), 'main/x'); + assert.strictEqual(instanceTask.state(0)[0], 5.0); + + // Running our instance again should use our new model changes. + + assert.ok(instance.run() > 0.0); + assert.strictEqual(instance.hasIssues, false); + + assert.strictEqual(instanceTask.state(0)[0], 9.0); + + model.removeChange(model.changes[1]); + + assert.ok(instance.run() > 0.0); + assert.strictEqual(instance.hasIssues, false); + + assert.strictEqual(instanceTask.state(0)[0], 7.0); + }); + + test('Change model after stop run', async () => { + // Note: stopping a run doesn't wait for it to be done, so our model may get changed while the stopped run is still + // in progress, which must not affect it. + + const NUMBER_OF_RUNS = 1000; + + const file = new loc.File(utils.resourcePath('api/sed/variables_initialised_using_variables.cellml')); + + file.setContents(utils.fileContents(file.path)); + + const document = new loc.SedDocument(file); + const model = document.model(0); + const instance = document.instantiate(); + + for (let i = 0; i < NUMBER_OF_RUNS; ++i) { + assert.strictEqual(instance.startRun(), true); + + instance.stopRun(); + + model.removeAllChanges(); + model.addChange(new loc.SedChangeAttribute('main', 'k', String(i))); + + instance.waitForRun(); + + assert.strictEqual(instance.hasIssues, false); + + // Note: this has no C++ or Python equivalent since the threads of our previous runs can only be cleaned up when + // we return to the event loop, which we must do or we would eventually run out of memory. + + await sleep(0); + } + + // Running our instance again should use our last model change. + + assert.ok(instance.run() > 0.0); + assert.strictEqual(instance.hasIssues, false); + + const instanceTask = instance.tasks[0]; + + assert.strictEqual(instanceTask.stateName(0), 'main/x'); + assert.strictEqual(instanceTask.state(0)[0], NUMBER_OF_RUNS - 1); + }); + + test('Model changes that cannot be applied', () => { + // Note: model changes that cannot be applied are reported and ignored, but they don't prevent an instance from + // being run, whether they are present when the instance is created or added afterwards. + + const expectedInstantiationIssues = [ + [ + loc.Issue.Type.WARNING, + "Task instance | Change attribute: the variable 'unknown' in component 'main' could not be found and therefore could not be changed." + ] + ]; + const expectedRunIssues = [ + [ + loc.Issue.Type.WARNING, + "Task | Change attribute: the variable 'unknown' in component 'main' could not be found and therefore could not be changed." + ] + ]; + const expectedOtherRunIssues = [ + [ + loc.Issue.Type.WARNING, + "Task | Change attribute: the computed constant 'cc' in component 'main' cannot be changed. Only state variables and constants can be changed." + ] + ]; + + const file = new loc.File(utils.resourcePath('api/sed/variables_initialised_using_variables.cellml')); + + file.setContents(utils.fileContents(file.path)); + + const document = new loc.SedDocument(file); + const model = document.model(0); + + model.addChange(new loc.SedChangeAttribute('main', 'k', '5.0')); + model.addChange(new loc.SedChangeAttribute('main', 'unknown', '7.0')); + + const instance = document.instantiate(); + + assertIssues(loc, instance, expectedInstantiationIssues); + + assert.ok(instance.run() > 0.0); + assertIssues(loc, instance, expectedRunIssues); + + const instanceTask = instance.tasks[0]; + + assert.strictEqual(instanceTask.stateName(0), 'main/x'); + assert.strictEqual(instanceTask.state(0)[0], 5.0); + + // Removing our model change that cannot be applied and running our instance again should not report any issues. + + model.removeChange(model.changes[1]); + + assert.ok(instance.run() > 0.0); + assert.strictEqual(instance.hasIssues, false); + + assert.strictEqual(instanceTask.state(0)[0], 5.0); + + // Adding another model change that cannot be applied and running our instance again should report it. + + model.addChange(new loc.SedChangeAttribute('main', 'cc', '9.0')); + + assert.ok(instance.run() > 0.0); + assertIssues(loc, instance, expectedOtherRunIssues); + + assert.strictEqual(instanceTask.state(0)[0], 5.0); + + // Removing that model change and running our instance again should not report any issues. + + model.removeChange(model.changes[1]); + + assert.ok(instance.run() > 0.0); + assert.strictEqual(instance.hasIssues, false); + + assert.strictEqual(instanceTask.state(0)[0], 5.0); + }); + + test('Initialisation fixed by model changes', () => { + // Note: an initialisation issue (e.g., an NLA system that cannot be solved) doesn't prevent an instance from being + // run once it has been fixed (e.g., using some model changes), even if it was present when the instance was + // created. + + const expectedInstantiationIssues = [ + [ + loc.Issue.Type.ERROR, + 'Task instance | KINSOL: the NLA system could not be solved (it contains some Inf and/or NaN values).' + ] + ]; + const expectedRunIssues = [ + [ + loc.Issue.Type.ERROR, + 'Task | KINSOL: the NLA system could not be solved (it contains some Inf and/or NaN values).' + ] + ]; + const file = new loc.File(utils.resourcePath('api/sed/kinsol_with_inf_and_or_nan_values.cellml')); + + file.setContents(utils.fileContents(file.path)); + + const document = new loc.SedDocument(file); + const model = document.model(0); + const instance = document.instantiate(); + + assertIssues(loc, instance, expectedInstantiationIssues); + + assert.strictEqual(instance.run(), 0.0); + assertIssues(loc, instance, expectedRunIssues); + + // Adding some model changes that fix our NLA system and running our instance again should not report any issues. + + model.addChange(new loc.SedChangeAttribute('main', 'q_3', '1.0')); + model.addChange(new loc.SedChangeAttribute('main', 'q_4', '1.0')); + + assert.ok(instance.run() > 0.0); + assert.strictEqual(instance.hasIssues, false); + + const instanceTask = instance.tasks[0]; + + assert.strictEqual(instanceTask.stateName(2), 'main/q_3'); + assert.strictEqual(instanceTask.state(2)[0], 1.0); + + // Removing those model changes and running our instance again should report our initialisation issue again. + + model.removeAllChanges(); + + assert.strictEqual(instance.run(), 0.0); + assertIssues(loc, instance, expectedRunIssues); + }); + test('Run while asynchronous run in progress', () => { // Note: running an instance while it is already being run asynchronously should wait for the asynchronous run to // complete before running the instance again. @@ -1370,7 +1581,7 @@ test.describe('Sed instance tests', () => { assertIssues(loc, instance, [ [ loc.Issue.Type.ERROR, - 'Task instance | KINSOL: the upper half-bandwidth cannot be equal to -1. It must be between 0 and 0.' + 'Task | KINSOL: the upper half-bandwidth cannot be equal to -1. It must be between 0 and 0.' ] ]); diff --git a/tests/bindings/javascript/solver.coverage.test.js b/tests/bindings/javascript/solver.coverage.test.js index 2b9287af2..620947f64 100644 --- a/tests/bindings/javascript/solver.coverage.test.js +++ b/tests/bindings/javascript/solver.coverage.test.js @@ -29,16 +29,7 @@ test.describe('Solver coverage tests', () => { }); test('ODE changes', () => { - const file = new loc.File(utils.resourcePath('api/solver/ode_sed_changes.omex')); - - file.setContents(utils.fileContents(file.path)); - - const document = new loc.SedDocument(file); - const instance = document.instantiate(); - - instance.run(); - - assertIssues(loc, instance, [ + const expectedInstantiationIssues = [ [ loc.Issue.Type.WARNING, "Task instance | Change attribute: the variable of integration 'time' in component 'environment' cannot be changed. Only state variables and constants can be changed." @@ -55,7 +46,59 @@ test.describe('Solver coverage tests', () => { loc.Issue.Type.WARNING, "Task instance | Change attribute: the algebraic variable 'i_Stim' in component 'membrane' cannot be changed. Only state variables and constants can be changed." ] - ]); + ]; + const expectedRunIssues = [ + [ + loc.Issue.Type.WARNING, + "Task | Change attribute: the variable of integration 'time' in component 'environment' cannot be changed. Only state variables and constants can be changed." + ], + [ + loc.Issue.Type.WARNING, + "Task | Change attribute: the variable 'X' in component 'membrane' could not be found and therefore could not be changed." + ], + [ + loc.Issue.Type.WARNING, + "Task | Change attribute: the computed constant 'E_Na' in component 'sodium_channel' cannot be changed. Only state variables and constants can be changed." + ], + [ + loc.Issue.Type.WARNING, + "Task | Change attribute: the algebraic variable 'i_Stim' in component 'membrane' cannot be changed. Only state variables and constants can be changed." + ] + ]; + const file = new loc.File(utils.resourcePath('api/solver/ode_sed_changes.omex')); + + file.setContents(utils.fileContents(file.path)); + + const document = new loc.SedDocument(file); + const instance = document.instantiate(); + + assertIssues(loc, instance, expectedInstantiationIssues); + + // Running our instance should report the same issues, but our changes that could not be applied should have been + // ignored rather than prevent our instance from being run, i.e. our change that could be applied should have been + // applied. + + assert.ok(instance.run() > 0.0); + assertIssues(loc, instance, expectedRunIssues); + + const instanceTask = instance.tasks[0]; + + assert.strictEqual(instanceTask.stateName(0), 'membrane/V'); + assert.strictEqual(instanceTask.state(0)[0], 3.0); + + // Running our instance again should report the same issues (rather than accumulate them). + + assert.ok(instance.run() > 0.0); + assertIssues(loc, instance, expectedRunIssues); + + // Instantiating our document again should report the same issues (rather than accumulate them). + + const otherInstance = document.instantiate(); + + assertIssues(loc, otherInstance, expectedInstantiationIssues); + + assert.ok(otherInstance.run() > 0.0); + assertIssues(loc, otherInstance, expectedRunIssues); }); test('Algebraic changes', () => { diff --git a/tests/bindings/python/test_sed_coverage.py b/tests/bindings/python/test_sed_coverage.py index f6b0ae542..2c34b27ba 100644 --- a/tests/bindings/python/test_sed_coverage.py +++ b/tests/bindings/python/test_sed_coverage.py @@ -648,12 +648,18 @@ def test_sed_uniform_time_course(): def test_sed_instance_and_sed_instance_task_differential_model(): - expected_issues = [ + expected_instantiation_issues = [ [ loc.Issue.Type.Error, "Task instance | CVODE: the upper half-bandwidth cannot be equal to -1. It must be between 0 and 3.", ], ] + expected_run_issues = [ + [ + loc.Issue.Type.Error, + "Task | CVODE: the upper half-bandwidth cannot be equal to -1. It must be between 0 and 3.", + ], + ] file = loc.File(utils.resource_path("api/solver/ode.cellml")) document = loc.SedDocument(file) @@ -665,6 +671,8 @@ def test_sed_instance_and_sed_instance_task_differential_model(): instance = document.instantiate() instance_task = instance.tasks[0] + assert_issues(instance, expected_instantiation_issues) + assert instance.has_tasks assert instance.task_count == 1 assert instance.task(0) == instance_task @@ -723,7 +731,7 @@ def test_sed_instance_and_sed_instance_task_differential_model(): instance.run() - assert_issues(instance, expected_issues) + assert_issues(instance, expected_run_issues) def test_sed_instance_and_sed_instance_task_non_differential_model(): diff --git a/tests/bindings/python/test_sed_instance.py b/tests/bindings/python/test_sed_instance.py index 748286044..9bcd40289 100644 --- a/tests/bindings/python/test_sed_instance.py +++ b/tests/bindings/python/test_sed_instance.py @@ -842,6 +842,208 @@ def test_simulation_settings_used_when_starting_run(): assert instance_task.voi[-1] == 2.0 * output_end_time +def test_model_changes_used_when_starting_run(): + # Note: the model changes used by a run are those in effect when the run is started, even if they get changed while + # the run is in progress. + + file = loc.File( + utils.resource_path("api/sed/variables_initialised_using_variables.cellml") + ) + document = loc.SedDocument(file) + model = document.model(0) + change = loc.SedChangeAttribute("main", "k", "5.0") + + model.add_change(change) + + instance = document.instantiate() + + assert instance.start_run() is True + + change.new_value = "7.0" + model.add_change(loc.SedChangeAttribute("main", "x", "9.0")) + + assert instance.wait_for_run() > 0.0 + assert not instance.has_issues + + instance_task = instance.tasks[0] + + assert instance_task.state_name(0) == "main/x" + assert instance_task.state(0)[0] == 5.0 + + # Running our instance again should use our new model changes. + + assert instance.run() > 0.0 + assert not instance.has_issues + + assert instance_task.state(0)[0] == 9.0 + + model.remove_change(model.changes[1]) + + assert instance.run() > 0.0 + assert not instance.has_issues + + assert instance_task.state(0)[0] == 7.0 + + +def test_change_model_after_stop_run(): + # Note: stopping a run doesn't wait for it to be done, so our model may get changed while the stopped run is still + # in progress, which must not affect it. + + NUMBER_OF_RUNS = 1000 + + file = loc.File( + utils.resource_path("api/sed/variables_initialised_using_variables.cellml") + ) + document = loc.SedDocument(file) + model = document.model(0) + instance = document.instantiate() + + for i in range(NUMBER_OF_RUNS): + assert instance.start_run() is True + + instance.stop_run() + + model.remove_all_changes() + model.add_change(loc.SedChangeAttribute("main", "k", str(i))) + + instance.wait_for_run() + + assert not instance.has_issues + + # Running our instance again should use our last model change. + + assert instance.run() > 0.0 + assert not instance.has_issues + + instance_task = instance.tasks[0] + + assert instance_task.state_name(0) == "main/x" + assert instance_task.state(0)[0] == NUMBER_OF_RUNS - 1 + + +def test_model_changes_that_cannot_be_applied(): + # Note: model changes that cannot be applied are reported and ignored, but they don't prevent an instance from + # being run, whether they are present when the instance is created or added afterwards. + + expected_instantiation_issues = [ + [ + loc.Issue.Type.Warning, + "Task instance | Change attribute: the variable 'unknown' in component 'main' could not be found and therefore could not be changed.", + ], + ] + expected_run_issues = [ + [ + loc.Issue.Type.Warning, + "Task | Change attribute: the variable 'unknown' in component 'main' could not be found and therefore could not be changed.", + ], + ] + expected_other_run_issues = [ + [ + loc.Issue.Type.Warning, + "Task | Change attribute: the computed constant 'cc' in component 'main' cannot be changed. Only state variables and constants can be changed.", + ], + ] + + file = loc.File( + utils.resource_path("api/sed/variables_initialised_using_variables.cellml") + ) + document = loc.SedDocument(file) + model = document.model(0) + + model.add_change(loc.SedChangeAttribute("main", "k", "5.0")) + model.add_change(loc.SedChangeAttribute("main", "unknown", "7.0")) + + instance = document.instantiate() + + assert_issues(instance, expected_instantiation_issues) + + assert instance.run() > 0.0 + assert_issues(instance, expected_run_issues) + + instance_task = instance.tasks[0] + + assert instance_task.state_name(0) == "main/x" + assert instance_task.state(0)[0] == 5.0 + + # Removing our model change that cannot be applied and running our instance again should not report any issues. + + model.remove_change(model.changes[1]) + + assert instance.run() > 0.0 + assert not instance.has_issues + + assert instance_task.state(0)[0] == 5.0 + + # Adding another model change that cannot be applied and running our instance again should report it. + + model.add_change(loc.SedChangeAttribute("main", "cc", "9.0")) + + assert instance.run() > 0.0 + assert_issues(instance, expected_other_run_issues) + + assert instance_task.state(0)[0] == 5.0 + + # Removing that model change and running our instance again should not report any issues. + + model.remove_change(model.changes[1]) + + assert instance.run() > 0.0 + assert not instance.has_issues + + assert instance_task.state(0)[0] == 5.0 + + +def test_initialisation_fixed_by_model_changes(): + # Note: an initialisation issue (e.g., an NLA system that cannot be solved) doesn't prevent an instance from being + # run once it has been fixed (e.g., using some model changes), even if it was present when the instance was + # created. + + expected_instantiation_issues = [ + [ + loc.Issue.Type.Error, + "Task instance | KINSOL: the NLA system could not be solved (it contains some Inf and/or NaN values).", + ], + ] + expected_run_issues = [ + [ + loc.Issue.Type.Error, + "Task | KINSOL: the NLA system could not be solved (it contains some Inf and/or NaN values).", + ], + ] + + file = loc.File( + utils.resource_path("api/sed/kinsol_with_inf_and_or_nan_values.cellml") + ) + document = loc.SedDocument(file) + model = document.model(0) + instance = document.instantiate() + + assert_issues(instance, expected_instantiation_issues) + + assert instance.run() == 0.0 + assert_issues(instance, expected_run_issues) + + # Adding some model changes that fix our NLA system and running our instance again should not report any issues. + + model.add_change(loc.SedChangeAttribute("main", "q_3", "1.0")) + model.add_change(loc.SedChangeAttribute("main", "q_4", "1.0")) + + assert instance.run() > 0.0 + assert not instance.has_issues + + instance_task = instance.tasks[0] + + assert instance_task.state_name(2) == "main/q_3" + assert instance_task.state(2)[0] == 1.0 + + # Removing those model changes and running our instance again should report our initialisation issue again. + + model.remove_all_changes() + + assert instance.run() == 0.0 + assert_issues(instance, expected_run_issues) + + def test_run_while_asynchronous_run_in_progress(): # Note: running an instance while it is already being run asynchronously should wait for the asynchronous run to # complete before running the instance again. @@ -1231,12 +1433,18 @@ def test_nla_model_with_no_nla_solver(): def test_dae_model(): - expected_issues = [ + expected_instantiation_issues = [ [ loc.Issue.Type.Error, "Task instance | KINSOL: the upper half-bandwidth cannot be equal to -1. It must be between 0 and 0.", ], ] + expected_run_issues = [ + [ + loc.Issue.Type.Error, + "Task | KINSOL: the upper half-bandwidth cannot be equal to -1. It must be between 0 and 0.", + ], + ] file = loc.File(utils.resource_path("api/sed/dae.cellml")) document = loc.SedDocument(file) @@ -1248,11 +1456,11 @@ def test_dae_model(): instance = document.instantiate() - assert_issues(instance, expected_issues) + assert_issues(instance, expected_instantiation_issues) instance.run() - assert_issues(instance, expected_issues) + assert_issues(instance, expected_run_issues) kinsol.linear_solver = loc.SolverKinsol.LinearSolver.Dense diff --git a/tests/bindings/python/test_solver_coverage.py b/tests/bindings/python/test_solver_coverage.py index c46629f62..4be8e65f1 100644 --- a/tests/bindings/python/test_solver_coverage.py +++ b/tests/bindings/python/test_solver_coverage.py @@ -20,7 +20,7 @@ def test_ode_changes(): - expected_issues = [ + expected_instantiation_issues = [ [ loc.Issue.Type.Warning, "Task instance | Change attribute: the variable of integration 'time' in component 'environment' cannot be changed. Only state variables and constants can be changed.", @@ -38,14 +38,56 @@ def test_ode_changes(): "Task instance | Change attribute: the algebraic variable 'i_Stim' in component 'membrane' cannot be changed. Only state variables and constants can be changed.", ], ] + expected_run_issues = [ + [ + loc.Issue.Type.Warning, + "Task | Change attribute: the variable of integration 'time' in component 'environment' cannot be changed. Only state variables and constants can be changed.", + ], + [ + loc.Issue.Type.Warning, + "Task | Change attribute: the variable 'X' in component 'membrane' could not be found and therefore could not be changed.", + ], + [ + loc.Issue.Type.Warning, + "Task | Change attribute: the computed constant 'E_Na' in component 'sodium_channel' cannot be changed. Only state variables and constants can be changed.", + ], + [ + loc.Issue.Type.Warning, + "Task | Change attribute: the algebraic variable 'i_Stim' in component 'membrane' cannot be changed. Only state variables and constants can be changed.", + ], + ] file = loc.File(utils.resource_path("api/solver/ode_sed_changes.omex")) document = loc.SedDocument(file) instance = document.instantiate() - instance.run() + assert_issues(instance, expected_instantiation_issues) + + # Running our instance should report the same issues, but our changes that could not be applied should have been + # ignored rather than prevent our instance from being run, i.e. our change that could be applied should have been + # applied. + + assert instance.run() > 0.0 + assert_issues(instance, expected_run_issues) + + instance_task = instance.tasks[0] + + assert instance_task.state_name(0) == "membrane/V" + assert instance_task.state(0)[0] == 3.0 + + # Running our instance again should report the same issues (rather than accumulate them). + + assert instance.run() > 0.0 + assert_issues(instance, expected_run_issues) + + # Instantiating our document again should report the same issues (rather than accumulate them). + + other_instance = document.instantiate() + + assert_issues(other_instance, expected_instantiation_issues) - assert_issues(instance, expected_issues) + assert other_instance.run() > 0.0 + assert_issues(other_instance, expected_run_issues) def test_algebraic_changes():