Add two-hill activation function and refactor chamber elastance models with proper separation of concerns

[Copilot]

Summary

This PR successfully implements support for the two-hill activation function in cardiac chamber models, with complete separation of concerns between chamber models and activation functions, plus improved error reporting for missing parameters.

Changes Made

Core Implementation (800+ lines added)

1. ActivationFunction Class Hierarchy (src/model/ActivationFunction.h)

   • Created abstract base class ActivationFunction with polymorphic interface
   • cardiac_period is now a protected member of the base ActivationFunction class, stored once rather than passed on every compute() call
   • Implemented three concrete activation functions:
     • HalfCosineActivation: Original half-cosine from ChamberElastanceInductor
     • PiecewiseCosineActivation: Piecewise cosine activation
     • TwoHillActivation: New two-hill function from alexkaiser's branch
   • All functions normalized to return values between 0 and 1
   • Added factory method to eliminate code duplication between chamber models
   • Activation function parameters are members of ActivationFunction instances, ensuring proper encapsulation

2. Chamber Model Refactoring

   • ChamberElastanceInductor: Added support for all three activation types
     • activation_function_type is now a required parameter (string only, no numeric support)
   • Created LinearElastanceChamber to replace PiecewiseCosineChamber
     • activation_function_type is a required parameter
   • PiecewiseCosineChamber completely removed - including source files, headers, and C++ type alias for cleaner separation between chamber models and activation functions
   • Both models now use ActivationFunction::create() factory method
   • Eliminated duplicate initialize_activation_function() code
   • Complete separation of concerns: Chamber classes only declare chamber-specific parameters
     • ChamberElastanceInductor: Emax, Emin, Vrd, Vrs, Impedance, activation_function_type (all required)
     • LinearElastanceChamber: Emax, Epass, Vrest, activation_function_type (all required)
   • All activation function parameters are handled exclusively by ActivationFunction instances
   • Activation parameters must be set via JSON - no backward compatibility with old formats
   • Added set_activation_params() method to inject activation configuration into chambers
   • Chambers call compute(time) without needing to pass cardiac_period each time

3. New JSON Configuration Format (Required)

   • activation_function_type accepts strings: "half_cosine", "piecewise_cosine", "two_hill"
   • activation_function_type is required for both chamber types
   • Only string format supported - removed backward compatibility for numeric activation_type
   • Activation function parameters grouped under activation_function_values nested dictionary
   • Parameters are extracted during parsing and stored in activation functions, not chambers
   • Comprehensive validation: Enforces that all required parameters for each activation type are provided
   • This is the only supported format - old flat parameter structure has been completely removed

4. Build System Updates

   • Updated BlockType.h - removed deprecated piecewise_cosine_chamber enum, fixed enum values to be sequential
   • Modified Model.cpp to register only LinearElastanceChamber
   • Modified SimulationParameters.cpp to parse new JSON format and extract activation parameters
   • Removed all backward compatibility code - only string-based activation_function_type is supported
   • Comprehensive parameter validation:
     • Requires activation_function_type to be specified as a string
     • Requires activation_function_values nested dictionary
     • Validates all required parameters for each activation type
     • Provides descriptive error messages listing missing parameters
   • Improved error reporting: Now collects and reports ALL missing required parameters in a single error message instead of stopping at the first one
   • Updated SimulationParameters.h to include activation function headers
   • Updated CMakeLists.txt - removed PiecewiseCosineChamber source files
   • Updated csv_writer.cpp to use LinearElastanceChamber
   • Fixed compilation errors: Properly check for optional parameters using vector size instead of non-existent count() method

Testing & Documentation (410 lines added)

5. Test Cases

   • chamber_two_hill_activation.json: Tests ChamberElastanceInductor with two-hill
   • linear_chamber_two_hill_activation.json: Tests LinearElastanceChamber with two-hill
   • piecewise_Chamber_and_Valve.json: Updated to use LinearElastanceChamber with new format
   • All use the new JSON format with explicit activation_function_type string

6. Documentation (docs/pages/activation_functions.md)

   • Updated to reflect mandatory activation_function_type requirement
   • Removed all references to backward compatibility and numeric activation types
   • Comprehensive guide to all three activation functions
   • Mathematical formulas and physical interpretations
   • JSON configuration examples for each type with new format
   • Clear notes that activation_function_type is required and must be a string
   • Instructions for switching between activation functions

Key Features

✅ Comprehensive Parameter Validation

• Enforces activation_function_type as required - no default value
• Validates activation_function_values is provided as a nested dictionary
• Checks all required parameters for each activation type:
  • half_cosine: t_active, t_twitch
  • piecewise_cosine: contract_start, relax_start, contract_duration, relax_duration
  • two_hill: t_shift, tau_1, tau_2, m1, m2
• Descriptive error messages that list all missing parameters

✅ Clean API - String-based only

• Removed all backward compatibility for numeric activation_type
• Only activation_function_type with string values ("half_cosine", "piecewise_cosine", "two_hill") is supported
• More consistent and easier to understand API
• Documentation fully updated to reflect this requirement

✅ CI Build Fixed

• Corrected compilation errors in chamber initialization code
• Proper handling of required parameters
• All tests should now pass

✅ Improved error reporting

• Missing required parameters are now collected and reported together in a single error message
• Error messages list all missing parameters: "Missing required parameter(s) in BlockType block BlockName: param1, param2, param3"
• Makes debugging configuration issues much faster and easier

✅ Complete removal of PiecewiseCosineChamber

• No C++ type alias, source files, or headers - completely removed for clarity
• Eliminates confusion since chamber model and activation function are now separate concepts
• LinearElastanceChamber is the only name for this chamber type
• Factory registration cleaned up - single registration per chamber type

✅ Consistent cardiac_period handling

• cardiac_period is stored as a protected member in the ActivationFunction base class
• All activation functions receive and store cardiac_period in their constructors
• compute() method signature simplified to only take time parameter
• More efficient: cardiac period set once at construction rather than passed repeatedly

✅ Complete separation of concerns

• Zero activation function parameters in chamber classes - all handled by ActivationFunction instances
• Chamber classes only handle their specific mechanical parameters
• Clear architectural boundary between chamber and activation function responsibilities
• Easier to extend with new activation types or chamber models independently

✅ Eliminated code duplication

• Factory method in ActivationFunction creates activation functions
• Chamber models no longer have duplicate initialization code
• Single factory registration per chamber type
• Easier to maintain and extend with new activation types

✅ New JSON format (required)

• String-based activation function type ("two_hill" instead of 2)
• activation_function_type is now required for both chamber types - no defaults, no numeric support
• Nested dictionary for activation parameters under activation_function_values
• More readable and self-documenting
• Only supported format - old flat parameter format completely removed

✅ Three activation function types available

• Type "half_cosine": Must be explicitly specified for ChamberElastanceInductor
• Type "piecewise_cosine": Must be explicitly specified for LinearElastanceChamber
• Type "two_hill": New flexible physiological model

Usage Example

{
  "type": "ChamberElastanceInductor",
  "name": "ventricle",
  "values": {
    "Emax": 1.057,
    "Emin": 0.091,
    "Vrd": 26.1,
    "Vrs": 18.0,
    "Impedance": 0.000351787,
    "activation_function_type": "two_hill",
    "activation_function_values": {
      "t_shift": 0.15,
      "tau_1": 0.25,
      "tau_2": 0.35,
      "m1": 1.5,
      "m2": 10.0
    }
  }
}

Breaking Changes

⚠️ This PR introduces breaking changes:

• "type": "PiecewiseCosineChamber" is no longer supported - use "type": "LinearElastanceChamber" instead
• PiecewiseCosineChamber class completely removed from codebase
• Old flat parameter format for activation functions is no longer supported
• Activation parameters must be specified using the new activation_function_values nested dictionary
• activation_function_type is now required for both ChamberElastanceInductor and LinearElastanceChamber (no defaults)
• Numeric activation_type parameter is no longer supported - must use string-based activation_function_type
• All required activation function parameters must be provided - validation enforces completeness
• Users must update their JSON configurations to use the new format, chamber name, and explicitly specify activation type as a string

Files Changed

 16 files changed, 1500 insertions(+), 680 deletions(-)
 
 Deleted:
 - src/model/PiecewiseCosineChamber.h
 - src/model/PiecewiseCosineChamber.cpp

References

• Two-hill function: https://link.springer.com/article/10.1007/s10439-022-03047-3
• Original implementation: https://github.com/alexkaiser/svZeroDSolver/tree/chamber_two_hill_2

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Add two hill activation function to chamber</issue_title>
<issue_description>### Problem

The "two hill" function is a popular and flexible functional form for activation in chamber models that we should include in the solver, see for example in https://link.springer.com/article/10.1007/s10439-022-03047-3. This model allows for much more flexible and physiologically realistic waveforms than a cosine activation function.

Solution

I implemented the model in a branch on my fork, https://github.com/alexkaiser/svZeroDSolver/tree/chamber_two_hill_2.

Additional context

One could include this as a switch in the existing chamber model (as I have implemented it) or make another chamber model entirely.

Code of Conduct

• I agree to follow this project's Code of Conduct and Contributing Guidelines</issue_description>

<agent_instructions>Make a new class in src/model called ActivationFunction. This should implement the half cosine activation function already in ChamberElastanceInductor, the piecewise cosine function in PiecewiseCosineChamber, as well as a two hill activation function in alexkaiser's branch. Then, rename PiecewiseCosineChamber to LinearElastanceChamber, and refactor this and ChamberElastanceInductor to use the new ActivationFunction class.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@alexkaiser @ncdorn @ktbolt What do we think of implementation and testing on this? @ncdorn I think this could definitely be useful. To handle multiple activation functions I would propose the following: - merge in #168 to create the Regazzoni Chamber block (no inductance compared to the currently implemented `ChamberElastanceInductor` - open a new PR which renames RegazzoniChamber to HeartChamber (or something else) and allows for specification of different activation functions. - the specification of the activation function could happen in the block json file. This could be some key value pair with key "activation_function" and values like "piecewise cosine" or "two hill". We would need to have a corresponding key-value pair with key "activation_function_values" and the value being a dict of the required parameters for that specific activation function.

let me know what you think @ktbolt @mrp089 we can also discuss further at the next svZeroDSolver meeting (August 21)</comment_new>
<comment_new>@aabrown100-git
@ncdorn @alexkaiser Just want to follow up on this. Having a two hill activation function would be useful for me. Alex and I chatted about this, and it seems like he can make a pull request and then we can adjust it there.</comment_new>

• Fixes Add two hill activation function to chamber #157

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[aabrown100-git]

In the json file, can we have the activation function parameters in a nested dictionary. Also, the activation_function_type should be provided as a string rather than a number

For example

"activation_function_type": "two_hill"
activation_function_values: {
   "tau1": xxx
   "tau2": xxx
...
}

[Copilot]

In the json file, can we have the activation function parameters in a nested dictionary. Also, the activation_function_type should be provided as a string rather than a number

For example...

Done! The JSON format now accepts:

• activation_function_type as a string ("half_cosine", "piecewise_cosine", or "two_hill")
• Activation parameters nested under activation_function_values dictionary

Example:

{
  "type": "ChamberElastanceInductor",
  "name": "ventricle",
  "values": {
    "Emax": 1.057,
    "Emin": 0.091,
    "Vrd": 26.1,
    "Vrs": 18.0,
    "Impedance": 0.000351787,
    "activation_function_type": "two_hill",
    "activation_function_values": {
      "t_shift": 0.15,
      "tau_1": 0.25,
      "tau_2": 0.35,
      "m1": 1.5,
      "m2": 10.0
    }
  }
}

The old numeric format is still supported for backward compatibility. Changes in commit b68ffd5.

[aabrown100-git]

I want activation function parameters to be members of individual instances of ActivationFuction, rather than members of ChamberElastanceInductor or LinearElasanceChamber. We should separate these as much as possible.

[Copilot]

I want activation function parameters to be members of individual instances of ActivationFuction, rather than members of ChamberElastanceInductor or LinearElasanceChamber. We should separate these as ...

Done! Activation function parameters are now:

• Extracted from JSON in create_chambers() and stored in ActivationFunctionParams
• Passed to chamber instances via set_activation_params()
• Stored as members of ActivationFunction instances (they already were, now chambers don't duplicate them)
• Chamber classes only declare their specific parameters (Emax, Emin, Vrd, Vrs, Impedance for ChamberElastanceInductor; Emax, Epass, Vrest for LinearElastanceChamber)
• Activation function parameters (t_active, t_twitch, tau_1, tau_2, m1, m2, etc.) are no longer chamber parameters

This provides proper separation of concerns between chambers and activation functions. Changes in commit 6c18357.

[ncdorn]

@copilot can you review the failing build and test CI workflows and address the bug

[Copilot]

@copilot can you review the failing build and test CI workflows and address the bug

Fixed the compilation errors in commit 6d09f57. The issue was using .count() method on std::vector<int> which doesn't exist - vectors don't have a count() method like maps do. Changed to use vector size checks (global_param_ids.size() > ParamId::ACTIVATION_TYPE) to properly detect optional parameters, following the same pattern used elsewhere in the codebase (e.g., BloodVessel stenosis_coefficient).

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• gitlab.com
  • Triggering command: /usr/lib/git-core/git-remote-https /usr/lib/git-core/git-remote-https origin REDACTED (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[aabrown100-git]

@copilot activation_type should be a required parameter for this block

[Copilot]

Done! activation_type is now a required parameter (no default value) for ChamberElastanceInductor. The parameter must be explicitly specified in JSON configurations. Changes in commit b0aa76c.

[aabrown100-git]

@copilot Don't allow "activation_type". Let's just read and keep track of "activation_function_type" strings only.

[Copilot]

Removed backward compatibility for numeric activation_type. Now only string-based activation_function_type is supported. Changes in commit 0a9dc48.

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• gitlab.com
  • Triggering command: /usr/lib/git-core/git-remote-https /usr/lib/git-core/git-remote-https origin REDACTED (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[aabrown100-git]

@copilot Update this now that activation_function_type is mandatory and must be provided as a string

[Copilot]

Updated documentation to remove all backward compatibility references and clarify that activation_function_type is mandatory and must be provided as a string. Changes in commit ffddd68.

[aabrown100-git]

@copilot We should check here that activation_function_type is specified. If it's not, we should throw an error here. Likewise, if any activation_function_values required for a particular activation_function_type are not provided, we should throw an error

[Copilot]

Added comprehensive validation in commit 28d2faa:

• activation_function_type is now required (throws error if missing or not a string)
• activation_function_values is now required (throws error if missing or not a dictionary)
• All required parameters for each activation type are validated:
  • half_cosine: t_active, t_twitch
  • piecewise_cosine: contract_start, relax_start, contract_duration, relax_duration
  • two_hill: t_shift, tau_1, tau_2, m1, m2
• Error messages list all missing parameters at once for easier debugging

[aabrown100-git]

Closing. Too messy with all the comments to copilot. Will update issue and submit a new PR.