Conversation
Adds support for time-dependent boundary conditions specified as mathematical function expressions (e.g. "2.0 * pi * COS(2.0 * pi * t)") using the exprtk header-only library. This enables FLOW and PRESSURE boundary conditions to accept an "fn" parameter as an alternative to providing Q/P and t arrays. Changes: - Add src/ThirdParty/exprtk.hpp expression parsing library - Extend Parameter class with function expression support - Add Model::add_parameter(string) for function-based parameters - Update SimulationParameters to parse "fn" string parameters - Make Q/P and t arrays optional in Flow/Pressure BCs when fn is used - Add test cases and reference results for time-dependent BCs - Add directed graph results for new test cases Agent-Logs-Url: https://github.com/SimVascular/svZeroDSolver/sessions/bf516764-19f7-45f0-b5af-9a031d3e9fc7 Co-authored-by: ncdorn <126125269+ncdorn@users.noreply.github.com>
|
@mrp089 figured I would have copilot take a shot at cleaning this up as it has been sitting as a draft PR for a while. let me know what you think. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #223 +/- ##
==========================================
- Coverage 79.80% 79.78% -0.03%
==========================================
Files 73 73
Lines 2902 2963 +61
Branches 337 349 +12
==========================================
+ Hits 2316 2364 +48
- Misses 586 599 +13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Two issues were causing CI failures:
1. BUILD FAILURE (macOS/Windows): Parameter.h included exprtk.hpp directly,
which caused -Wshadow warnings-as-errors for every compilation unit that
included Parameter.h (via Model.h). Fixed by using the PIMPL pattern:
- Moved #include "../ThirdParty/exprtk.hpp" from Parameter.h to Parameter.cpp
- Wrapped the include with #pragma diagnostic push/pop to suppress -Wshadow
- Added ExprtkImpl forward declaration and std::unique_ptr<ExprtkImpl> in
Parameter class (private section)
- Declared ~Parameter(), Parameter(Parameter&&), operator=(Parameter&&)
in header, defined as = default in Parameter.cpp after ExprtkImpl is complete
2. CODECHECK FAILURE (Prettier): timeDep_Flow.json and timeDep_Pressure.json
used 4-space indentation but Prettier requires 2-space. Also removed the
unused reference result files (result_timeDep_Flow.json,
result_timeDep_Pressure.json) which were not referenced by any test.
Agent-Logs-Url: https://github.com/SimVascular/svZeroDSolver/sessions/6f309553-a450-492b-ac6e-9ae45ea0b244
Co-authored-by: ncdorn <126125269+ncdorn@users.noreply.github.com>
Replace the bundled ThirdParty/exprtk.hpp (20240101) with the latest upstream release from ArashPartow/exprtk (20250101). The newer version includes bug fixes including: proper fallthrough annotations, to_str refactoring, hex_to_bin fix, and other improvements. The -Wshadow warnings emitted by exprtk.hpp's enum declarations are already suppressed in Parameter.cpp via #pragma diagnostic push/pop guards (added in the previous commit), so no additional changes are needed to handle compilation on macOS/Windows with strict warning flags. Agent-Logs-Url: https://github.com/SimVascular/svZeroDSolver/sessions/6fd01362-fe19-40aa-ad6c-d31deeb83c57 Co-authored-by: ncdorn <126125269+ncdorn@users.noreply.github.com>
- Add /bigobj flag for Parameter.cpp on MSVC to fix C1128 (too many COFF sections from exprtk.hpp template expansion) - Add doxygen comments to ExprtkImpl members to fix WARN_AS_ERROR failures from WARN_NO_PARAMDOC = YES in the Doxyfile - Add @return doc to Parameter::operator= (also triggered by WARN_NO_PARAMDOC) Agent-Logs-Url: https://github.com/SimVascular/svZeroDSolver/sessions/ccba344d-dc40-4f1f-b274-8b0e1f1b2977 Co-authored-by: ncdorn <126125269+ncdorn@users.noreply.github.com>
Agent-Logs-Url: https://github.com/SimVascular/svZeroDSolver/sessions/ee3b39c5-e833-4e2c-8afd-fc6b05df7675 Co-authored-by: ncdorn <126125269+ncdorn@users.noreply.github.com>
…tors Agent-Logs-Url: https://github.com/SimVascular/svZeroDSolver/sessions/032f4526-1c5d-459d-83e6-0835b4090927 Co-authored-by: ncdorn <126125269+ncdorn@users.noreply.github.com>
There was a problem hiding this comment.
this is useful as it allows future handling of string parameters which previously did not exist
ncdorn
left a comment
There was a problem hiding this comment.
this looks good to me. I think the changes are significant but it allows for future implementation of string parameters in blocks.
Agent-Logs-Url: https://github.com/SimVascular/svZeroDSolver/sessions/cb7d97f2-fced-4d42-8a6a-003e8d3c0f7b Co-authored-by: ncdorn <126125269+ncdorn@users.noreply.github.com>
Agent-Logs-Url: https://github.com/SimVascular/svZeroDSolver/sessions/cb7d97f2-fced-4d42-8a6a-003e8d3c0f7b Co-authored-by: ncdorn <126125269+ncdorn@users.noreply.github.com>
There was a problem hiding this comment.
@copilot instead of creating a dict in this code file, just load the appropriate json from the tests/cases directory and make the desired edit to the config key or value. this keeps the code much cleaner and more concise
There was a problem hiding this comment.
Updated the test to load the JSON case and edit the existing config value directly in commit 44d6541.
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: ncdorn <126125269+ncdorn@users.noreply.github.com>
…ft-pr-186 # Conflicts: # src/model/Block.h # src/model/CMakeLists.txt Co-authored-by: ncdorn <126125269+ncdorn@users.noreply.github.com>
Co-authored-by: ncdorn <126125269+ncdorn@users.noreply.github.com>
There was a problem hiding this comment.
we need to remove this file and just import the exprtk package instead of keeping the source code in the repo
|
|
||
| add_library(${lib} OBJECT ${CXXSRCS} ${HDRS}) | ||
|
|
||
| # exprtk.hpp generates too many COFF sections for the default MSVC object format |
There was a problem hiding this comment.
we should not need this change if we just import the exprtk package and remove the source code from this repo.
Co-authored-by: ncdorn <126125269+ncdorn@users.noreply.github.com>
Addressed in commit 68ecf5b: exprtk source was removed from the repo, exprtk is now fetched via CMake FetchContent, and the model target consumes it via |
Summary
Adds support for specifying boundary conditions using arbitrary time-dependent mathematical expressions (functions of
t), as an alternative to the existing tabular time-value arrays. This allows users to write closed-form formulas like2*pi*cos(2*pi*t)directly in the JSON input file instead of supplying discretetandQ/Parrays.Feature:
fnfield for flow and pressure boundary conditionsFlowReferenceBCandPressureReferenceBCnow accept an optionalfnfield containing a mathematical expression string. Whenfnis provided, the solver evaluates the expression at each time step to obtain the boundary condition value, bypassing thet/Qort/Parrays (which become optional whenfnis present).Example input:
{ "bc_type": "FLOW", "bc_values": { "fn": "2.0 * (4*ATAN(1.)) * COS(2.0 * (4*ATAN(1.)) * t)" } }Expression evaluation is powered by the exprtk library (vendored at
src/ThirdParty/exprtk.hpp, version 20250101). The library supports standard math operators, functions (sin,cos,exp,log, …), and built-in constants. Any compilation errors in the expression string are reported at model load time with detailed diagnostics before throwing.Implementation details
Parameter(src/model/Parameter.h/.cpp): Added a new constructor andupdate()overload that accept an expression string. Expression state is held in a PIMPL struct (ExprtkImpl) so that exprtk types stay out of the header, keeping compile times manageable.Parameter::get(double time)evaluates the compiled expression whenis_function == true. Explicit destructor, move constructor, and move assignment are defined in the.cppto support theunique_ptr<ExprtkImpl>member.InputParameter(src/model/Parameter.h): Added anis_functionflag so that the parameter-dispatch logic inSimulationParameters.cppknows to read a string rather than a numeric array.Model(src/model/Model.h/.cpp): Addedadd_parameter(const std::string expression_string)overload. Also added a fallback that setscardiac_cycle_period = 1.0when no cardiac cycle period can be inferred (needed for models that use only function-based BCs with no periodic arrays).SimulationParameters.cpp: Extendedgenerate_block()to handleis_functionparameters by reading a string field and calling the newModel::add_parameteroverload. Thetand value arrays are now treated as optional when a function is also declared.FlowReferenceBC/PressureReferenceBC: ThetandQ/Pparameters are now marked optional (is_optional = true); a new optionalfnparameter withis_function = trueis added to each block's parameter list.src/ThirdParty/exprtk.hpp. Included only inParameter.cpp(never in headers) and wrapped in#pragma diagnostic push/popto suppress-Wshadowwarnings from the third-party code. MSVC requires/bigobjforParameter.cppdue to exprtk's template expansion, added viaset_source_files_propertiesinsrc/model/CMakeLists.txt.Tests
tests/cases/timeDep_Flow.jsonandtimeDep_Pressure.json: Two new minimal test models (RC vessel, one inlet and one outlet) that drive the boundary condition withfn = "2*pi*cos(2*pi*t)".tests/test_io.py::test_time_dependent_block: Verifies that the computed flow and pressure at the inlet match the analytical expression to within 0.3% relative tolerance after the initial transient settles.