From 06f62175a4991e99deb66b0e16b8ca0f26ba8c44 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Fri, 1 Dec 2023 23:22:33 +0100 Subject: [PATCH 1/2] Refactor event handling (#2216) Just extracting 2 functions. No change in functionality. --- include/amici/forwardproblem.h | 15 ++++ src/forwardproblem.cpp | 138 +++++++++++++++++---------------- 2 files changed, 88 insertions(+), 65 deletions(-) diff --git a/include/amici/forwardproblem.h b/include/amici/forwardproblem.h index dfe3bd8f22..91e5d50791 100644 --- a/include/amici/forwardproblem.h +++ b/include/amici/forwardproblem.h @@ -250,6 +250,21 @@ class ForwardProblem { void handleEvent(realtype* tlastroot, bool seflag, bool initial_event); + /** + * @brief Store pre-event model state + * + * @param seflag Secondary event flag + * @param initial_event initial event flag + */ + void store_pre_event_state(bool seflag, bool initial_event); + + /** + * @brief Check for, and if applicable, handle any secondary events + * + * @param tlastroot pointer to the timepoint of the last event + */ + void handle_secondary_event(realtype* tlastroot); + /** * @brief Extract output information for events */ diff --git a/src/forwardproblem.cpp b/src/forwardproblem.cpp index 72fd87c979..13946547ef 100644 --- a/src/forwardproblem.cpp +++ b/src/forwardproblem.cpp @@ -182,6 +182,77 @@ void ForwardProblem::handleEvent( if (model->nz > 0) storeEvent(); + store_pre_event_state(seflag, initial_event); + + if (!initial_event) + model->updateHeaviside(roots_found_); + + applyEventBolus(); + + if (solver->computingFSA()) { + /* compute the new xdot */ + model->fxdot(t_, x_, dx_, xdot_); + applyEventSensiBolusFSA(); + } + + handle_secondary_event(tlastroot); + + /* only reinitialise in the first event fired */ + if (!seflag) { + solver->reInit(t_, x_, dx_); + if (solver->computingFSA()) { + solver->sensReInit(sx_, sdx_); + } + } +} + +void ForwardProblem::storeEvent() { + if (t_ == model->getTimepoint(model->nt() - 1)) { + // call from fillEvent at last timepoint + model->froot(t_, x_, dx_, rootvals_); + for (int ie = 0; ie < model->ne; ie++) { + roots_found_.at(ie) = (nroots_.at(ie) < model->nMaxEvent()) ? 1 : 0; + } + root_idx_.push_back(roots_found_); + } + + if (getRootCounter() < getEventCounter()) { + /* update stored state (sensi) */ + event_states_.at(getRootCounter()) = getSimulationState(); + } else { + /* add stored state (sensi) */ + event_states_.push_back(getSimulationState()); + } + + /* EVENT OUTPUT */ + for (int ie = 0; ie < model->ne; ie++) { + /* only look for roots of the rootfunction not discontinuities */ + if (nroots_.at(ie) >= model->nMaxEvent()) + continue; + + /* only consider transitions false -> true or event filling */ + if (roots_found_.at(ie) != 1 + && t_ != model->getTimepoint(model->nt() - 1)) { + continue; + } + + if (edata && solver->computingASA()) + model->getAdjointStateEventUpdate( + slice(dJzdx_, nroots_.at(ie), model->nx_solver * model->nJ), ie, + nroots_.at(ie), t_, x_, *edata + ); + + nroots_.at(ie)++; + } + + if (t_ == model->getTimepoint(model->nt() - 1)) { + // call from fillEvent at last timepoint + // loop until all events are filled + fillEvents(model->nMaxEvent()); + } +} + +void ForwardProblem::store_pre_event_state(bool seflag, bool initial_event) { /* if we need to do forward sensitivities later on we need to store the old * x and the old xdot */ if (solver->getSensitivityOrder() >= SensitivityOrder::first) { @@ -212,18 +283,9 @@ void ForwardProblem::handleEvent( xdot_disc_.push_back(xdot_); xdot_old_disc_.push_back(xdot_old_); } +} - if (!initial_event) - model->updateHeaviside(roots_found_); - - applyEventBolus(); - - if (solver->computingFSA()) { - /* compute the new xdot */ - model->fxdot(t_, x_, dx_, xdot_); - applyEventSensiBolusFSA(); - } - +void ForwardProblem::handle_secondary_event(realtype* tlastroot) { int secondevent = 0; /* check whether we need to fire a secondary event */ @@ -260,60 +322,6 @@ void ForwardProblem::handleEvent( ); handleEvent(tlastroot, true, false); } - - /* only reinitialise in the first event fired */ - if (!seflag) { - solver->reInit(t_, x_, dx_); - if (solver->computingFSA()) { - solver->sensReInit(sx_, sdx_); - } - } -} - -void ForwardProblem::storeEvent() { - if (t_ == model->getTimepoint(model->nt() - 1)) { - // call from fillEvent at last timepoint - model->froot(t_, x_, dx_, rootvals_); - for (int ie = 0; ie < model->ne; ie++) { - roots_found_.at(ie) = (nroots_.at(ie) < model->nMaxEvent()) ? 1 : 0; - } - root_idx_.push_back(roots_found_); - } - - if (getRootCounter() < getEventCounter()) { - /* update stored state (sensi) */ - event_states_.at(getRootCounter()) = getSimulationState(); - } else { - /* add stored state (sensi) */ - event_states_.push_back(getSimulationState()); - } - - /* EVENT OUTPUT */ - for (int ie = 0; ie < model->ne; ie++) { - /* only look for roots of the rootfunction not discontinuities */ - if (nroots_.at(ie) >= model->nMaxEvent()) - continue; - - /* only consider transitions false -> true or event filling */ - if (roots_found_.at(ie) != 1 - && t_ != model->getTimepoint(model->nt() - 1)) { - continue; - } - - if (edata && solver->computingASA()) - model->getAdjointStateEventUpdate( - slice(dJzdx_, nroots_.at(ie), model->nx_solver * model->nJ), ie, - nroots_.at(ie), t_, x_, *edata - ); - - nroots_.at(ie)++; - } - - if (t_ == model->getTimepoint(model->nt() - 1)) { - // call from fillEvent at last timepoint - // loop until all events are filled - fillEvents(model->nMaxEvent()); - } } void ForwardProblem::handleDataPoint(int /*it*/) { From dff4ec3b0bdca26b1e8c019eaa54158b4669239e Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Sun, 3 Dec 2023 15:11:33 +0100 Subject: [PATCH 2/2] GHA: Fix actions for PRs from forks (#2219) * run codecov only on pull requests * trigger relevant actions on `pull_request` instead of only on `push` * disables sonar-scanner if the upload token is not available Closes #2218 --- .github/workflows/deploy_branch.yml | 2 +- .github/workflows/test_install.yml | 2 +- .github/workflows/test_petab_test_suite.yml | 1 + .github/workflows/test_python_cplusplus.yml | 7 +++++++ .github/workflows/test_sbml_semantic_test_suite.yml | 1 + .github/workflows/test_windows.yml | 1 + 6 files changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/deploy_branch.yml b/.github/workflows/deploy_branch.yml index 73294286a1..82d1452d37 100644 --- a/.github/workflows/deploy_branch.yml +++ b/.github/workflows/deploy_branch.yml @@ -1,5 +1,5 @@ name: Deploy Branch -on: [push, merge_group, workflow_dispatch] +on: [push, pull_request, merge_group, workflow_dispatch] jobs: sdist: diff --git a/.github/workflows/test_install.yml b/.github/workflows/test_install.yml index be74cfa4c6..166616b434 100644 --- a/.github/workflows/test_install.yml +++ b/.github/workflows/test_install.yml @@ -1,5 +1,5 @@ name: Installation -on: [push, merge_group, workflow_dispatch] +on: [push, pull_request, merge_group, workflow_dispatch] jobs: archive: diff --git a/.github/workflows/test_petab_test_suite.yml b/.github/workflows/test_petab_test_suite.yml index d5c4dc4fe8..df188e7abe 100644 --- a/.github/workflows/test_petab_test_suite.yml +++ b/.github/workflows/test_petab_test_suite.yml @@ -85,6 +85,7 @@ jobs: tests/petab_test_suite/ - name: Codecov + if: github.event_name == 'pull_request' || github.repository_owner == 'AMICI-dev' uses: codecov/codecov-action@v3.1.0 with: token: ${{ secrets.CODECOV_TOKEN }} diff --git a/.github/workflows/test_python_cplusplus.yml b/.github/workflows/test_python_cplusplus.yml index a531d2db2c..496779c6ec 100644 --- a/.github/workflows/test_python_cplusplus.yml +++ b/.github/workflows/test_python_cplusplus.yml @@ -6,6 +6,7 @@ on: pull_request: branches: - master + - develop jobs: ubuntu-cpp-python-tests: @@ -65,6 +66,7 @@ jobs: ${AMICI_DIR}/python/tests/test_splines.py - name: Codecov Python + if: github.event_name == 'pull_request' || github.repository_owner == 'AMICI-dev' uses: codecov/codecov-action@v3.1.0 with: token: ${{ secrets.CODECOV_TOKEN }} @@ -84,6 +86,7 @@ jobs: && lcov -a coverage_cpp.info -a coverage_py.info -o coverage.info - name: Codecov CPP + if: github.event_name == 'pull_request' || github.repository_owner == 'AMICI-dev' uses: codecov/codecov-action@v3.1.0 with: token: ${{ secrets.CODECOV_TOKEN }} @@ -92,6 +95,7 @@ jobs: fail_ci_if_error: true - name: Run sonar-scanner + if: ${{ env.SONAR_TOKEN != '' }} env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} @@ -137,6 +141,7 @@ jobs: ${AMICI_DIR}/python/tests/test_splines_short.py - name: Codecov Python + if: github.event_name == 'pull_request' || github.repository_owner == 'AMICI-dev' uses: codecov/codecov-action@v3.1.0 with: token: ${{ secrets.CODECOV_TOKEN }} @@ -156,6 +161,7 @@ jobs: && lcov -a coverage_cpp.info -a coverage_py.info -o coverage.info - name: Codecov CPP + if: github.event_name == 'pull_request' || github.repository_owner == 'AMICI-dev' uses: codecov/codecov-action@v3.1.0 with: token: ${{ secrets.CODECOV_TOKEN }} @@ -164,6 +170,7 @@ jobs: fail_ci_if_error: true - name: Run sonar-scanner + if: ${{ env.SONAR_TOKEN != '' }} env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} diff --git a/.github/workflows/test_sbml_semantic_test_suite.yml b/.github/workflows/test_sbml_semantic_test_suite.yml index 0fde56b8f9..2af34d3762 100644 --- a/.github/workflows/test_sbml_semantic_test_suite.yml +++ b/.github/workflows/test_sbml_semantic_test_suite.yml @@ -54,6 +54,7 @@ jobs: path: tests/amici-semantic-results - name: Codecov SBMLSuite + if: github.event_name == 'pull_request' || github.repository_owner == 'AMICI-dev' uses: codecov/codecov-action@v3.1.0 with: token: ${{ secrets.CODECOV_TOKEN }} diff --git a/.github/workflows/test_windows.yml b/.github/workflows/test_windows.yml index 53834c3000..9dc13efbea 100644 --- a/.github/workflows/test_windows.yml +++ b/.github/workflows/test_windows.yml @@ -7,6 +7,7 @@ on: - cron: '48 4 * * *' pull_request: branches: + - develop - master jobs: