-
Notifications
You must be signed in to change notification settings - Fork 171
Add extra variables for easier check of derivative calculations of table blocks #4529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Is this related to ContinuousClock is not suitable for testing derivatives in ModelicaTest.Tables #4325?
- Should the comparisonSignals.txt files be adapted to actually track the new signals?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK for me.
WalkthroughThe pull request introduces new variables, parameters, and equations in several Modelica files. In the Changes
Sequence Diagram(s)sequenceDiagram
participant DT as d_t_new
participant TD as TestDer
participant Fac as factor1
DT->>TD: Provide u (initialize integrated)
DT->>TD: Send y for derivative calculation
TD->>TD: Compute der(integrated) = Fac * y
sequenceDiagram
participant DT as d_t_new
participant D2T as d2_t_new
participant TD2 as TestDer2
participant Fac as factor1
DT->>TD2: Provide u (initialize integrated2)
TD2->>TD2: Compute der(integrated2) = Fac * der_integrated2
D2T->>TD2: Send y for second derivative (der(der_integrated2))
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (18)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
The summary by coderabbit misses the intent as far I can tell, and a text " In the TestDer models, a new real variable integrated is added (initialized with d_t_new.u) along with a parameter factor1 to manage symbolic simplification" is misleading as the intent is to "avoid" (not "manage") symbolic simplifications. |
In order to check the derivative-approximation a simple possibility is to just integrate it and see that it is roughly the same as the original. It doesn't replace the reference results, but it helps in analyzing potential errors.
As an example simulate
ModelicaTest.Tables.CombiTable1Ds.Test34
and plotd_t_new.u
,integrated
,integrated2
. They should all be almost identical to each other. The parameter withEvaluate=false
is added so that tools don't have the amazing idea of seeing through the differentiation combined with integration. There is no need to have references for these variables.Possible extensions:
Summary by CodeRabbit