Skip to content

Support Moshi wildcards via dispatch #946

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 1 commit into from
Mar 4, 2025
Merged

Conversation

visr
Copy link
Contributor

@visr visr commented Feb 27, 2025

Fixes #945
Reported upstream as Roger-luo/Moshi.jl#43.

This should restore the old behavior. One possible exception is is_discrete_time_domain, which was defined as not being continuous. So that previously returned true for non-clocks, which seems undesirable to me. Though I'd be happy to preserve that behavior if it is expected anywhere.

@visr
Copy link
Contributor Author

visr commented Feb 28, 2025

Ha I missed #944, which already fixes #945. I just rebased this on top of that because it's still worth fixing this behavior on master, which is unrelated to the Moshi change:

julia> is_discrete_time_domain(:banana)
true

And it is probably a bit better to use dispatch here instead of isa.

@AayushSabharwal
Copy link
Member

Ha I missed #944, which already fixes #945.

Haha yeah this PR had me confused for a minute, "Wait didn't this already merge? And wasn't the author someone else?"

@AayushSabharwal
Copy link
Member

While I agree it does make sense that is_discrete_time_domain return false for non-time-domain arguments, it has been the way it is for a long time. I'm concerned this may break things that somehow rely on the weird behavior. MTK CI isn't running right now - SciML/ModelingToolkit.jl#3382 will fix that and once it passes I'll approve this.

@ChrisRackauckas ChrisRackauckas merged commit 7771788 into SciML:master Mar 4, 2025
59 of 102 checks passed
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.

isclock is not robust to non TimeDomain argument
3 participants