Skip to content

starting support of continuous NonLinModel (RK4 only for now) #38

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

Merged
merged 13 commits into from
Mar 14, 2024

Conversation

baggepinnen
Copy link
Member

I'm opening the PR to allow commenting

Copy link
Member Author

@baggepinnen baggepinnen left a comment

Choose a reason for hiding this comment

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

If a solver is provided, is the dynamics automatically interpreted in continuous time then? Or in other words, the way the user communicates whether the dynamics is continuous or discrete time is by the presence of absence of the solver?

@franckgaga
Copy link
Member

franckgaga commented Mar 9, 2024

If a solver is provided, is the dynamics automatically interpreted in continuous time then? Or in other words, the way the user communicates whether the dynamics is continuous or discrete time is by the presence of absence of the solver?

Yes exactly! If the user provides a solver, the dynamics are assumed continuous, else discrete. No solver is the default behavior to avoid breaking changes. But, since the package is still in v0.Y.Z, I'm not closed to the idea of introducing a breaking change by defaulting to continuous-time nonlinear model with e.g.: solver=RungeKutta(4). Nonlinear continuous-time model are way more common in practice (e.g. almost all first principle models)

@baggepinnen
Copy link
Member Author

No solver is the default behavior to avoid breaking changes. But, since the package is still in v0.Y.Z

Keep in mind that as long as the major version is 0, every minor release (change of Y above) is considered a breaking release, so this package has already made 17 breaking releases which would have forced anyone depending on this package to update their dependency compat and make new releases.

@franckgaga
Copy link
Member

No solver is the default behavior to avoid breaking changes. But, since the package is still in v0.Y.Z

Keep in mind that as long as the major version is 0, every minor release (change of Y above) is considered a breaking release, so this package has already made 17 breaking releases which would have forced anyone depending on this package to update their dependency compat and make new releases.

From https://semver.org/#spec-item-4, it seems that there is no specific rule for breaking releases in major version 0.

I think I will make a breaking change and default to continuous time model, with solver=RungeKutta(4). This is more common in practice and this is the behaviour of most MPC pakage incl. MATLAB toolbox.

@baggepinnen
Copy link
Member Author

From https://semver.org/#spec-item-4, it seems that there is no specific rule for breaking releases in major version 0.

Julia isn't following semver proper, the julia version is detailed here https://pkgdocs.julialang.org/v1/compatibility/#compat-pre-1.0

@franckgaga
Copy link
Member

franckgaga commented Mar 14, 2024

Thanks for the info, I did not know that! I will follow this rule from now on.

I will register a new minor version soon with the breaking change above. I now use the default RK4 solver in the pendulum example of the manual.

@franckgaga franckgaga merged commit 90369ca into main Mar 14, 2024
3 of 4 checks passed
@franckgaga franckgaga deleted the continuous_nonlinmodel branch March 14, 2024 14:06
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.

2 participants