Time dependency for DiracSum and ElementStep#901
Open
kmokstad wants to merge 4 commits into
Open
Conversation
kmokstad
changed the title
There was a problem hiding this comment.
Pull request overview
This PR restores/extends time-dependent behavior for DiracSum and ElementSteps (moved out of the Darcy app) by allowing amplitudes to be specified as expressions in time t. It also wires setParam() through the involved function wrappers so external code can update parameters used inside expression functions.
Changes:
- Allow
DiracSum/ElementStepsto accept an amplitude string (constant or time-expression) and buildSpaceTimeFuncwhere applicable. - Add/propagate
setParam()support throughScalarFunc,ConstTimeFunc,SpaceTimeFunc, expression functions, andFunctionSum. - Refactor expression-function implementation to share parsing/evaluation state (
ExpressionHolder) and unify parse error tracking (ExprEval::numError).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Utility/Functions.h | Makes ConstTimeFunc/SpaceTimeFunc own non-const subfunctions and forwards setParam() to them. |
| src/Utility/Functions.C | Updates parsing logic to use unified expression error counter and non-const function pointers. |
| src/Utility/Function.h | Adds ScalarFunc::setParam() to support externally-set expression parameters. |
| src/Utility/ExprFunctions.h | Introduces ExpressionHolder, adds setParam() plumbing to expression functions, and refactors derivative handling. |
| src/Utility/ExprFunctions.C | Implements ExpressionHolder, moves to ExprEval::numError, and updates evaluation/derivative code paths. |
| src/SIM/FunctionSum.h | Adds FunctionSum::setParam() and RealFuncSum::addFuncComp() to support time-dependent amplitudes. |
| src/SIM/FunctionSum.C | Implements parameter forwarding and time-expression amplitude handling for DiracSum via SpaceTimeFunc. |
| src/SIM/ElementSteps.h | Extends constructor to accept a geometric tolerance parameter (eps). |
| src/SIM/ElementSteps.C | Updates parsing to accept amplitude strings and route time-dependent amplitudes via SpaceTimeFunc. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Added: Override setParam() method in FunctionSum.
class EvalFuncScalar. Also override setParam() in the classes ConstTimeFunc and SpaceTimeFunc forwarding to their ScalarFunc and RealFunc members (with const removed). Added: Convenience function utl::isTimeExpression().
containing the ExprEval stuff that the two classes EvalFuncScalar and EvalFuncSpatial have in common, do reduce code duplications, etc. Add setParam() override for EvalFuncScalar.
Consider any string which not entirely can be converted to a floating point value as expression, such that setParam() works also if the expression is a user-defined constant parameter with no time dependency.
Collaborator
Author
|
jenkins build this with downstreams please |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This should restore the functionality probably gone (although no tests for it) when moving these two
FunctionSumsub-classes out from the Darcy app. Expressions int(time) can now be specified as amplitude instead of a constant, and aSpaceTimeFuncobject will then be created instead where the time function is a scalar expression function. AlsosetParam()can be used in case the function expressions contain parameters what can be set externally.