Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

refactor: move MTKStdlib to subpackage#3999

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
AayushSabharwal wants to merge6 commits intomaster
base:master
Choose a base branch
Loading
fromas/mtkstdlib

Conversation

@AayushSabharwal
Copy link
Member

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 theSciML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

Additional context

Add any other context about the problem here.

@AayushSabharwal
Copy link
MemberAuthor

I'm not sure why CI isn't running. Will also figure out a way to do this so I don't get the credit for MTKStdlib

@AayushSabharwalAayushSabharwal marked this pull request as draftOctober 29, 2025 09:28
@github-actions
Copy link
Contributor

github-actionsbot commentedOct 29, 2025
edited
Loading

Benchmark Results (Julia vlts)

Time benchmarks
master5df2a20...master /5df2a20...
ODEProblem0.0935 ± 0.0075 s0.0978 ± 0.0057 s0.956 ± 0.095
init0.0591 ± 0.0022 ms0.0582 ± 0.0016 ms1.01 ± 0.048
large_parameter_init/ODEProblem0.985 ± 0.0044 s0.985 ± 0.01 s1 ± 0.011
large_parameter_init/init0.111 ± 0.003 ms0.11 ± 0.0024 ms1.01 ± 0.035
mtkcompile0.0345 ± 0.0015 s0.0344 ± 0.0015 s1 ± 0.061
time_to_load5.2 ± 0.11 s5.21 ± 0.06 s0.998 ± 0.024
Memory benchmarks
master5df2a20...master /5df2a20...
ODEProblem0.656 M allocs: 22 MB0.667 M allocs: 22.4 MB0.981
init0.892 k allocs: 0.0461 MB0.892 k allocs: 0.0461 MB1
large_parameter_init/ODEProblem4.17 M allocs: 0.163 GB4.14 M allocs: 0.161 GB1.01
large_parameter_init/init1.74 k allocs: 0.0857 MB1.74 k allocs: 0.0857 MB1
mtkcompile0.225 M allocs: 8.36 MB0.224 M allocs: 8.33 MB1
time_to_load0.153 k allocs: 14.5 kB0.153 k allocs: 14.5 kB1

@github-actions
Copy link
Contributor

github-actionsbot commentedOct 29, 2025
edited
Loading

Benchmark Results (Julia v1)

Time benchmarks
master5df2a20...master /5df2a20...
ODEProblem0.0976 ± 0.0055 s0.0966 ± 0.007 s1.01 ± 0.093
init0.0442 ± 0.011 ms0.0449 ± 0.011 ms0.983 ± 0.34
large_parameter_init/ODEProblem1.13 ± 0.025 s1.13 ± 0.039 s1 ± 0.041
large_parameter_init/init0.077 ± 0.018 ms0.0769 ± 0.016 ms1 ± 0.32
mtkcompile0.0342 ± 0.0012 s0.0338 ± 0.00043 s1.01 ± 0.037
time_to_load4.78 ± 0.051 s4.62 ± 0.071 s1.03 ± 0.019
Memory benchmarks
master5df2a20...master /5df2a20...
ODEProblem0.668 M allocs: 22 MB0.681 M allocs: 22.4 MB0.98
init0.797 k allocs: 30.6 kB0.797 k allocs: 30.6 kB1
large_parameter_init/ODEProblem4.39 M allocs: 0.161 GB4.39 M allocs: 0.161 GB1
large_parameter_init/init1.64 k allocs: 0.0596 MB1.64 k allocs: 0.0596 MB1
mtkcompile0.231 M allocs: 7.9 MB0.23 M allocs: 7.88 MB1
time_to_load0.149 k allocs: 11.1 kB0.149 k allocs: 11.1 kB1

@ChrisRackauckas
Copy link
Member

I don't think we want to keep most of these libraries. Most of the Dyad versions are also open sourced and at this point better maintained. Which of the sublibraries here are not covered by the main Dyad component libraries? It seems it might just be a documentation thing where we should just link to all component libraries out there in the ecosystem, which should include the Dyad ones but also AI4E or whichever ones people develop.

@baggepinnen I know you have opinions on this, are there any that we should bring "in house" to MTK here?

@AayushSabharwal
Copy link
MemberAuthor

I don't particularly mind what we keep in MTKStdlib. My motivation for moving this here is that any breaking release has to jump through too many hoops because tests depend on MTKStdlib. We can recommend users use Dyad libraries, but I strongly oppose the use of Dyad models in our tests.

@ChrisRackauckas
Copy link
Member

@baggepinnen which standard libraries are worth preserving? Copying conversation into here,@SebastianM-C mentioned:

Dyad is very opinionated on what representation to pick. MtKStdLib has things like Mechanical.Translational, MechanicalPosition, MechanicalModelica

@baggepinnen

Getting rid of all of those in favor of the Dyad (modelica) one would be welcome

And yes, I agree that having 3 mutually incompatible versions of the Mechanical library, where 2 are effectively pretty broken in most scenarios, doesn't serve anyone. Since Dyad's MechanicalComponents.jl is a more complete version of MechanicalModelica and it's BSD-3 licensed like the Modelica Standard Library, there is 0 possible advantage to having a separate MechanicalModelica since it's just a less maintained version of it. But while we're at it, we might as well remove the two bad ones. So that means all of the mechanical components should be removed and we should just tell people to use MechanicalComponents.jl

But is every library like that? ElectricalComponents.jl as well. etc. Multibody2D: should we separate that out to a different repo? Is it being maintained? It's just aLink component and nothing else?

From a cursory look by me, it looks like the only thing to really keep then is the digital components from ElectricalComponents should be moved to ElectricalComponents.jl. Maybe some of the block components to. But we don't want anything else out of these libraries at this point if I'm not mistaken?

I strongly oppose the use of Dyad models in our tests.

If we need to have small versions of ElectricalComponents or something as a sublibrary like ModelingToolkitTestComponents.jl in order to make testing easier, that's fine. But that's very different from having two versions of the standard library (or with mechanical components, 4 😅 where we tell people to not use 3 of them!) Just from a signaling standpoint, I think 99% of people are just better off if we tell them to use the one that's maintained, which is not the one there. That repo has been a bit of a trap.

@baggepinnen
Copy link
Contributor

Multibody2D: should we separate that out to a different repo? Is it being maintained? It's just a Link component and nothing else?

Remove it, connectors there do not transmit torque so it's pretty broken. Multibody.jl has a complete version of planar mechanics.

There have been some improvements to some components that aren't present in modelica stdlib, but I think we should just add those to dyad as well

@hersle
Copy link
Contributor

At least "Basic Blocks" libraries andInterpolation,ParametrizedInterpolation,SampledData, ... components sound domain-independent and generically useful to me and makes sense to have in a standard library.

Electrical/magnetic/mechanical/thermal/... components sound more specialized and domain-specific to me, and could make sense both inside/outside a standard library, depending on what one is going for.

@ChrisRackauckas
Copy link
Member

At least "Basic Blocks" libraries and Interpolation, ParametrizedInterpolation, SampledData, ... components sound domain-independent and generically useful to me and makes sense to have in a standard library.

Yes, these should live as a sublibrary here in alib folder. That will keep them better maintained and stop them from breaking 😅 . ModelingToolkitNeuralNets.jl should arguably be a lib folder too.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@AayushSabharwal@ChrisRackauckas@baggepinnen@hersle

[8]ページ先頭

©2009-2025 Movatter.jp