Skip to content
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

Fix derivatives in 2D-table for one-sided extrapolation by constant continuation #3896

Merged
merged 2 commits into from
Dec 29, 2021

Conversation

beutlich
Copy link
Member

Closes #3894.

@GarronFish You can test by cloning master branch of ModelicaTableAdditions where I already updated the binaries.

@beutlich beutlich added bug Critical/severe issue L: C-Sources Issue addresses Modelica/Resources/C-Sources labels Nov 12, 2021
@beutlich beutlich added this to the MSL4.1.0 milestone Nov 12, 2021
@beutlich beutlich self-assigned this Nov 12, 2021
@GarronFish
Copy link

Closes #3894.

@GarronFish You can test by cloning master branch of ModelicaTableAdditions where I already updated the binaries.

I've tested the functionality using tabletest4.txt attached.
tabletest4.txt

The input signals go "around" the table 0,1,2
1,1,2
2.2,3
to test the derivative calculation for each case.
The test gave the expected results.

Functionally this appears to be working correctly now. Thanks!

@beutlich
Copy link
Member Author

I've tested the functionality using tabletest4.txt attached.

Nice stimulation. I added it as new test model. Thanks.

Functionally this appears to be working correctly now. Thanks!

Thanks for testing and confirmation.

@GarronFish
Copy link

I've tested the functionality using tabletest4.txt attached.

Nice stimulation. I added it as new test model. Thanks.

Functionally this appears to be working correctly now. Thanks!

Thanks for testing and confirmation.

The test is better if the table is set so the derivative values are unique for example:
table=[0,1,2; 1,1,7; 2,2,4],

@HansOlsson
Copy link
Contributor

I'm having a bit of trouble understanding how this is supposed to work. Basically I expected the getValue code to be matched by derivative code, but the codes are different and I only see how the new code matches linear interpolation in table and hold-extrapolation - but not for other interpolations for hold-interpolation. Am I missing something obvious?

@beutlich
Copy link
Member Author

The change considers the cases to HOLD_LAST_POINT where there is one-sided extrapolation.

Copy link
Contributor

@HansOlsson HansOlsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Critical/severe issue L: C-Sources Issue addresses Modelica/Resources/C-Sources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Derivative signal of combiTable2Ds is set to zero outside of table with HoldLastPoint
4 participants