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

kappa calculation moved from initial equation to binding equation #4130

Merged

Conversation

AndreaBartolini
Copy link
Contributor

this fixes OpenModelica Issue #12677

@casella
Copy link
Contributor

casella commented Mar 5, 2025

@casella
Copy link
Contributor

casella commented Mar 5, 2025

Fixes OpenModelica/OpenModelica#12677. A small drawback of this solution is that the aggregationCellTimes() function is now called twice with the same argument, once in a binding equation for a parameter that is needed as input to compute kappa, another time in a surviving initial equation, which only computes a rCel[82] array, which has manageable dimensions.

I think OMC should be smart enough to figure out that they are the same function call and factor it out. Even if it doesn't, I understand the overhead of calling that function twice is marginal, considering that the extra computation is performed only once during initialization.

@mwetter mwetter changed the base branch from master to issue_omc_12677_initialEquation March 7, 2025 15:19
@mwetter mwetter merged commit a9469e0 into lbl-srg:issue_omc_12677_initialEquation Mar 7, 2025
2 of 3 checks passed
@mwetter
Copy link
Member

mwetter commented Mar 7, 2025

Thanks for the change. I will process it from here and merge it to the master branch.

mwetter added a commit that referenced this pull request Mar 10, 2025
* kappa calculation moved from initial equation to binding equation (#4130)

* Enabled models


---------

Co-authored-by: AndreaBartolini <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants