Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

pass ODESystem kwargs via @mtkmodel constructor #3484

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hexaeder
Copy link
Contributor

@hexaeder hexaeder commented Mar 21, 2025

I think it would be nice if you could pass kw args to the ODESystem constructor even if you system has been defined via @mtkmodel.
Motivating example:

    @mtkmodel UnitFailure begin
        @variables begin
            x(t), [unit = u"V"]
            y(t), [unit = u"A"]
        end
        @equations begin
            x ~ y
        end
    end
    UnitFailure(name = :uf, checks = false) # no error with passed kwarg

Checklist

  • Appropriate tests were added
  • Any code changes were done in a way that does not break public API
  • All documentation related to code changes were updated
  • The new code follows the
    contributor guidelines, in particular the SciML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

@hexaeder hexaeder marked this pull request as ready for review March 21, 2025 20:41
@baggepinnen
Copy link
Contributor

Is this really a good idea? What if pass UnitFailure(var_with_typo=val)? Will MTK not silently accept this and I get a confusing result due to having a typo in my variable name? IIRC, MTK accepts kwargs... in several internal functions making misspelled keyword arguments slip through unnoticed.

@hexaeder
Copy link
Contributor Author

Fair point, I was hoping that every keyword argument is eventually matched or errors, but its never silently dropped. I agree that this can be super frustrating.

Another way would be to make it explicit, i.e.

UnitFailure(name = :uf, odesystem=(; checks = false))

which would fail if somebody names their variables odesystem...

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants