Two hill activation function and separate activation function class by aabrown100-git · Pull Request #215 · SimVascular/svZeroDSolver

aabrown100-git · 2026-02-14T06:12:33Z

Current situation

Resolves #157

Release Notes

Separates chamber models and activation functions, which are now handled by a new class ActivationFunction and its derived classes HalfCosineActivation, PiecewiseCosineActivation, and TwoHillActivation
Activation functions can be specified in the input file for a chamber as, for example

"chambers": [
    {
      "type": "LinearElastanceChamber",
      "name": "LA",
      "values": {
        "Emax": 2.666e7,
        "Epass": 2.0449931e7,
        "Vrest": 4.0e-6
      },
      "activation_function": {
        "type": "two_hill",
        "t_shift": 0.025,
        "tau_1": 0.07586206896551724,
        "tau_2": 0.12413793103448276,
        "m1": 1.32,
        "m2": 13.1
      }
    },

Adds two hill activation function. Previously, half cosine and piecewise cosine activations were hardcoded into ChamberElastanceInductor and PiecewiseCosineChamber, respectively. But no chamber had a two hill activation function
Add closed loop test case for two hill activation function.
Rename PiecewiseCosineChamber to more approprate LinearElastanceChamber.

Documentation

All new code documented via Doxygen.

Testing

Existing tests chamber_elastance_inductor.json and Piecewise_Chamber_and_Valve.json still pass after refactoring
Added new test closed_loop_two_hill.json

Code of Conduct & Contributing Guidelines

I agree to follow the Code of Conduct and Contributing Guidelines.

…wise cosine, and two hill activation functions. ActivationFunction is created in SimulationParameters.cpp and associated with a chamber. Renamed PiecewiseCosineChamber to LinearElastanceChamber. All tests pass

Copilot

Pull request overview

This PR refactors chamber activation handling by introducing a dedicated ActivationFunction hierarchy (including a new two-hill activation), updating chamber models to consume it, and extending the test suite with a closed-loop two-hill case.

Changes:

Added ActivationFunction base class with HalfCosineActivation, PiecewiseCosineActivation, and TwoHillActivation.
Refactored chamber models to accept an activation_function JSON object (instead of hardcoded activation logic/params).
Added/updated test cases, including a new closed-loop two-hill regression input (and expected directed-graph output).

Reviewed changes

Copilot reviewed 19 out of 21 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`src/model/ActivationFunction.h`	Defines activation function class hierarchy + parameters schema.
`src/model/ActivationFunction.cpp`	Implements activation computations and two-hill normalization.
`src/model/LinearElastanceChamber.h`	Renames/refactors chamber model and adds activation function ownership.
`src/model/LinearElastanceChamber.cpp`	Uses `ActivationFunction` to compute time-varying elastance.
`src/model/ChamberElastanceInductor.h`	Removes hardcoded activation params and adds activation function ownership.
`src/model/ChamberElastanceInductor.cpp`	Uses `ActivationFunction` for activation computation.
`src/model/Block.h`	Adds `set_activation_function()` hook for chamber blocks.
`src/model/BlockType.h`	Updates/renames block type enum values for linear elastance chamber.
`src/model/Model.h`	Updates include to `LinearElastanceChamber`.
`src/model/Model.cpp`	Updates factory mapping from old chamber type string to `LinearElastanceChamber`.
`src/model/CMakeLists.txt`	Adds new activation function compilation units; swaps old chamber files for new ones.
`src/solve/SimulationParameters.h`	Declares `generate_activation_function()` loader helper.
`src/solve/SimulationParameters.cpp`	Implements activation function JSON parsing/validation and wiring into chambers.
`src/solve/csv_writer.cpp`	Updates type filter to include `LinearElastanceChamber` instead of the old chamber class.
`tests/test_solver.py`	Adds `closed_loop_two_hill.json` to the parameterized test list.
`tests/cases/chamber_elastance_inductor.json`	Moves activation params into `activation_function` object.
`tests/cases/piecewise_Chamber_and_Valve.json`	Renames chamber type and moves piecewise params into `activation_function`.
`tests/cases/closed_loop_two_hill.json`	New closed-loop regression case using `two_hill` activation.
`tests/cases/dirgraph-results/closed_loop_two_hill_directed_graph.dot`	New expected directed graph output for the new test case.
`.gitignore`	Adds Python virtualenv ignores (`venv/`, `.venv/`).

Comments suppressed due to low confidence (1)

src/model/LinearElastanceChamber.h:193

There are two consecutive private: labels here (private: repeated twice). Please remove the duplicate to avoid confusion and keep the access specifiers clean.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/model/ActivationFunction.cpp

src/solve/SimulationParameters.cpp

src/model/ChamberElastanceInductor.h

src/model/Block.h

src/model/ActivationFunction.h

…error on Windows. Validate correct inputs and behavior when computing two hill normalization factor. Avoid unused parameter warning in set_activation_function. Remove extra private labels

ktbolt · 2026-02-17T19:02:05Z

src/model/ActivationFunction.cpp

+
+ActivationFunction::ActivationFunction(
+    double cardiac_period,
+    std::vector<std::pair<std::string, InputParameter>> params)


@aabrown100-git Can this be changed to

const std::vector<std::pair<std::string, InputParameter>>& params)

or do you need to copy ?

I refactored how these parameters are handled. Now, this variable is passed by const ref, although it is immediately copied into a member variable

src/model/ActivationFunction.cpp

ktbolt · 2026-02-17T19:06:16Z

src/model/ActivationFunction.cpp

+}
+
+std::vector<std::pair<std::string, InputParameter>>
+ActivationFunction::get_input_params() const {


@aabrown100-git std::pair<std::string, InputParameter> is used a lot here; should you define that as a type ?

using ParameterPair = std::pair<std::string, InputParameter>;

Removed this function. now, std::pair<std::string, InputParameter> is used twice in ActivationFunction.h (same as in Block.h), and once in ActivationFunction.cpp. I think it's fine to leave it without defining as a type.

…ut parameter properties

ktbolt · 2026-02-17T22:54:32Z

src/model/ActivationFunction.cpp

-  const double t_active = params_.at("t_active").second;
-  const double t_twitch = params_.at("t_twitch").second;
+  const double t_active = params_.at("t_active");
+  const double t_twitch = params_.at("t_twitch");


@aabrown100-git Are t_active and t_twitch guaranteed to be in params_ ?

Yes, in two ways. They are initialized to default values (zeros) in the constructor ActivationFunction::ActivationFunction(). Also, if the user does not provide these parameters in the JSON file, SimulationParameters::generate_activation_function() will throw an error. So by the time we get to this function, these parameters are guaranteed to exist and map to values provided by the user.

ktbolt · 2026-02-17T23:03:30Z

@aabrown100-git I'm wondering about the checking of parameters in ActivationFunction for example

  if (!it->second.first.is_number) {
    throw std::runtime_error("ActivationFunction::set_param: parameter '" +
                             name + "' is not a scalar number");
  }

Aren't parameters checked where they are read in ?

aabrown100-git · 2026-02-17T23:12:21Z

@ktbolt Yes, you're right, checking the parameters in ActivationFunction is redundant. The validity of the parameter name and value checked in generate_activation_function(), which called ActivationFunction::set_param(). I can remove these redundant checks, or keep it to be defensive.

ktbolt · 2026-02-18T01:11:10Z

@aabrown100-git It is probably a good idea to remove redundancy. Later developers also look at current code as a template and checking parameters may imply that they need to be checked here.

… rely on validation in SimulationParameters::generate_activation_function, which calls set_param()

aabrown100-git · 2026-02-18T04:20:57Z

@ktbolt Done!

aabrown100-git added 6 commits February 13, 2026 15:40

Initial commit. Implements ActivationFunction with half cosine, piece…

24ff34a

…wise cosine, and two hill activation functions. ActivationFunction is created in SimulationParameters.cpp and associated with a chamber. Renamed PiecewiseCosineChamber to LinearElastanceChamber. All tests pass

Fix formatting

d4961ed

Add doxygen for cardiac_period_ and params_

a6c3a97

Add closed_loop_two_hill test case

5ea98dc

Add directed_graph for new test case

ca36c98

Add documentation and fix formatting

25f8a70

aabrown100-git requested review from Copilot, ktbolt, mrp089 and ncdorn February 14, 2026 06:12

aabrown100-git self-assigned this Feb 14, 2026

Copilot started reviewing on behalf of aabrown100-git February 14, 2026 06:13 View session

Copilot AI reviewed Feb 14, 2026

View reviewed changes

aabrown100-git added 2 commits February 13, 2026 22:46

Address copilot comments. Explicit define M_PI for to avoid compiler …

aca919e

…error on Windows. Validate correct inputs and behavior when computing two hill normalization factor. Avoid unused parameter warning in set_activation_function. Remove extra private labels

Fix formatting

aecb0c0

ktbolt reviewed Feb 17, 2026

View reviewed changes

src/model/ActivationFunction.cpp Outdated Show resolved Hide resolved

ktbolt reviewed Feb 17, 2026

View reviewed changes

Refactor to simplify parameter handling. Separate parameters from inp…

9affef4

…ut parameter properties

ktbolt reviewed Feb 17, 2026

View reviewed changes

Remove redundant activation parameter validation in set_param(). Now,…

0057457

… rely on validation in SimulationParameters::generate_activation_function, which calls set_param()

Conversation

aabrown100-git commented Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current situation

Release Notes

Documentation

Testing

Code of Conduct & Contributing Guidelines

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ktbolt Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

aabrown100-git Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ktbolt Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

aabrown100-git Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

ktbolt Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

aabrown100-git Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

ktbolt commented Feb 17, 2026

Uh oh!

aabrown100-git commented Feb 17, 2026

Uh oh!

ktbolt commented Feb 18, 2026

Uh oh!

aabrown100-git commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aabrown100-git commented Feb 14, 2026 •

edited

Loading