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

[BREAKING Don't merge] Support DynamicQuantities #2349

Merged
merged 14 commits into from
Jan 4, 2024
Merged

Conversation

YingboMa
Copy link
Member

No description provided.

@YingboMa YingboMa changed the title Support DynamicQuantities [BREAKING Don't merge] Support DynamicQuantities Nov 11, 2023
test/dq_units.jl Outdated
@test !MT.validate(bad_eqs)
@test !MT.validate(bad_length_eqs)
@named sys = ODESystem(good_eqs, t, [], [])
@test_throws MT.ValidationError ODESystem(bad_eqs, t, [], []; name = :sys)
Copy link
Member

Choose a reason for hiding this comment

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

This is supposed to pass. It seems it just converts but I presume the codegen doesn't add a convert?

Copy link
Member Author

Choose a reason for hiding this comment

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

We currently only allow SI units for DQ. The new commit should fix the CI failure.

@YingboMa
Copy link
Member Author

YingboMa commented Jan 3, 2024

I decided to keep iv in the unit check. Not checking iv will lead to a lot of special cases, and to have canonical t, the user should write

const t = ModelingToolkit.t_dynamic_quantities

or

const t = ModelingToolkit.t_unitful

or

const t = ModelingToolkit.t

@ChrisRackauckas
Copy link
Member

I see. Given the direction we're going, I'd propose:

t
t_nounits
t_unitful

D
D_nounits
D_unitful

@YingboMa
Copy link
Member Author

YingboMa commented Jan 3, 2024

To have a nicer syntax, we can have @import_independent_variables, @import_independent_variables Unitful, etc... Or a shorter variant of the name.

@YingboMa
Copy link
Member Author

YingboMa commented Jan 4, 2024

Because of SymbolicML/DynamicQuantities.jl#105, we cannot use DQ for the standard library.

@ChrisRackauckas ChrisRackauckas merged commit b0f8f78 into master Jan 4, 2024
14 of 22 checks passed
@ChrisRackauckas ChrisRackauckas deleted the myb/unit branch January 4, 2024 05:31
@ChrisRackauckas
Copy link
Member

Master is now setup for the v9, no releases till that.

@oscardssmith
Copy link
Contributor

for posterity, should the don't merge be removed from the title?

@MilesCranmer
Copy link
Contributor

MilesCranmer commented Jan 12, 2024

Just confirming that precompilation was fixed with SymbolicML/DynamicQuantities.jl#106.

The compat bound on DynamicQuantities you will want is on 0.11.2.

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.

4 participants