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

Update Continuous.mo #4257

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Update Continuous.mo #4257

wants to merge 2 commits into from

Conversation

hubertus65
Copy link
Member

Better suggestion to fix init issue for Integrators in Modelica.Blocks.Continuous. WARNING: @MartinOtter, I currently don't have a setup to test this immediately, so this is UNTESTED. Please test before committing. It would be nice to have a test when this is in a loop with a non-linear plant + steady-state initialization.

Better suggestion to fix init issue for Integrators in Modelica.Blocks.Continuous. WARNING: @MartinOtter, I currently don't have a setup to test this immediately, so this is UNTESTED. Please test before committing. It would be nice to have a test when this is in a loop with a non-linear plant + steady-state initialization.
@hubertus65 hubertus65 marked this pull request as draft January 12, 2024 17:17
Fixed failed syntax check.
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.

This will not work for two reasons:

  • The default for the start-attribute is now "empty", not 0.0 as in Modelica 3.4. Thus for non-steady-state there will be a new start-value, that may cause problems.
  • There are also initial equations that weren't modified, so there will now be both a start-value with fixed=true and an initial equation - which is even more problematic.

For Dymola this is likely to be worse than before, since we could previously down-prioritize a start-value if there was an identical (possibly inactive) initial equation (even if not standardized it could at least be explained) - now they differ so we cannot do that.

It may be possible to find a solution that only provides a non-fixed start-value when needed, I will check later.

Copy link
Member

@MartinOtter MartinOtter left a comment

Choose a reason for hiding this comment

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

As Hans explained, your proposed solution is not good.

A (non-backwards) compatible good solution would be, as performed in Rotational.Components. Inertia, where no start or initial equations are defined at all, but "showStartAttribute=true" is provided for the relevant variables often used for initialization:
grafik
This is much clearer and the user has a good GUI to select the most important initialization options (and then it is completely clear in the user interface which values are set with fixed = true, fixed = false or not set).

Since it seems overkill to make such a non-backwards compatible change now (which requires an appropriate, probably non-trivial, conversion script), I would suggest to keep the merged solution, but correct the description text for y_start:
Real y_start= 0 "Initial value of output (= state), if initType=initialState or initialOutout)"
instead of the current
Real y_start = 0 "Initial or guess value of output (= state)"

If the block appears in a nonlinear algebraic loop and y is an iteration variable, then, as usual, the user has to make a modifier (y(start=xxx)). One might also explain this in the documentation.

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.

None yet

3 participants