ModelicaStandardLibrary
ModelicaStandardLibrary copied to clipboard
Consider the derivatives at boundaries in the 2D-table
Issue: That is best described using the new test-model plot input and output of the derivative-blocks and see that the inputs don't change with time, but still ~~half of them~~ have spikes in their outputs (*).
Solution: If we are at boundaries between regions in the 2D-table we should ideally chose the branch we will be at in the future - or the one we were at in the past.
Detailed explanation: For a 2D-table the problem is that we can be at a double boundary and the solution e.g., goes C->B, but in the middle we could select A or D, and in those regions the derivatives differ for linear interpolation.
| A | B |
|---|---|
| C | D |
This also explains why I ignored it for the 1D-tables; as we cannot be in such a point between four regions in a 1D-table.
*: All four variants now have the same issue. They didn't originally.
It might need a better explanation, better motivating example, and the actual code could be made less impactful.
The example is now fully updated; and it turns out that there were more subtle differences for the cases.
The comment explains it:
- Case 2 and 4 slide diagonally (from different corners)
- Case 1 and 3 start at the edge of the real table and then leave it top-left - Interpolating in different ways
Typo on line 505: /* Same as findRowIndex2 but works on rows */
should be /* Same as findRowIndex2 but works on columns */
Just to be clear: That is an existing issue, I just blindly copy-pasted. (But it is now corrected.)
https://github.com/modelica/ModelicaStandardLibrary/blob/e5f0e7eb5050456a8040d5a847adaab939cf866b/Modelica/Resources/C-Sources/ModelicaStandardTables.c#L497
@HansOlsson Will you rebase your branch to resolve the conficts or shall I?
@HansOlsson Will you rebase your branch to resolve the conficts or shall I?
I can do it.
@HansOlsson I added some minor formatting corrections and hope you agree on.
We should squash the commits anyway before/during the merge.
@MartinOtter @sjoelund this PR has been stuck for about 2 months.