-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
Units don't seem to work correctly #287
Comments
Unit tracking doesn't currently work with PDEs. Waiting for SciML/MethodOfLines.jl#286 and SciML/MethodOfLines.jl#287 to fix
Running into the same issue - unit validation passes on the PDESystem but not on the discretized system. I can post an MWE for my case if it'd help |
What would be the successful case, that the result array has the right units? |
Potentially it'd be running unit validation on the PDESystem and trusting that that correctness is preserved on the discretization. The units in the result array being correct is ensured by the user declaring it as a variable with those units and it passing the PDESystem validation - it seems like it'd run just as correctly and with fewer hurdles if the discretized system didn't have to be unit checked (or we figure out autopromotion rules for constants to unit aware constants, but that seems harder) |
I guess the immediate successful case would be that the example above would run without error. As far as I can tell it's just an issue of the constants that are added being units (e.g. when the upwind scheme checks for u > 0, the 0 is unitless but u is not). |
Hello,
Here is a small example with units:
It works if there aren't any units, and as you can see the units on the two sides of the equation match. However, running the code to the end gives this error:
ERROR: ModelingToolkit.ValidationError("Some equations had invalid units. See warnings for details.")
Here are the warnings in question:
It looks like what is happening is that the upwind differencing is comparing the velocity to zero, but the zero doesn't have any units, which causes the problem. I also tried
advection_scheme=WENOScheme()
, but it gave a similar error.The text was updated successfully, but these errors were encountered: