[WIP] Unit conversions

[lamorton]

The primary goal here is to switch from checking the units to inserting conversion factors such that equations will be accepted as long as they have consistent dimensions. Other improvements include allowing to use Unitful literals in equations and allowing degrees for angles. Solves #1228.

This entails major changes to the way validation works. Basically, the workhorse is now constructunit which recursively rewrites an equation/expression so that the top-level object has units, and then get_unit is just _get_unit composed with this. (_get_unit only works if the top-level object has unit metadata on it.) The fact that the validate function now has to return expressions (used to return boolean for success/failure) pushed me back to using the exception-handling system all the way through, which I think will simplify things somewhat. Also, in order to provide for context in the error messages, instead of pushing context info down the function calls, I'm moving toward intercepting the rising error messages & tacking the context information onto them.

Tests for ODESystem and NonlinearSystem with units are passing, but I still need to:

• Convert remaining validate methods
• Implement the error message augmentation
• Documentation

I might try to refactor/condense/rename things a bit further as well.

[lamorton]

@isaacsas @ChrisRackauckas I'll try to minimize it but this is likely to break unit-related things in Catalyst.

[isaacsas]

@lamorton if you could help us fix up Catalyst once this is finalized that would be great.

I think this is a great functionality, but I wonder if it might make more sense to be an opt-in system transformation instead? I imagine this is going to have a major impact on the performance of system construction, and I'm also not sure about rewriting user equations by default. I guess we never benchmarked how much of an impact unit checking had, and don't yet know how much of a performance impact the new unit checking with rewriting will have.

As an alternative design, what about keeping the old unit approach, but giving a more informative error that says something like: "your system has inconsistent units in equation (GIVE EQUATION), please call balanceunits(sys) to let ModelingToolkit try to generate a new system with consistent units", thus making this new code an opt-in system transformation users can call to generate a new system with appropriate units? This seems more inline with how MT does system modification in other contexts too (i.e. structural_simplify is an opt-in choice for users to apply). Alternatively, maybe this could be an additional kwarg in system construction that allows users to switch between just having units checked vs. having them rewritten (if possible)?

Anyways just thought I'd make the suggestion. I can see how doing this automatically to make consistent units is nice too, but it just seems like once we are rewriting user equations we are essentially generating a new system, so might as well make this a transformation.

[lamorton]

@isaacsas Hmm, that's an interesting point about making the rewrite an optional transformation. That would also remove the need to robustly handle every system with units from the outset. Also, it might avoid breaking anything downstream. I'll definitely help fix whatever problems this causes downstream, in any event.

The motivation for making this the default is that if you were to write the equations non-symbolically they'd already be able to handle the unit conversions. For instance, if I modelingtoolkitize a function that would have been able to correctly add centimeters + meters, I've now lost that capability until I perform the rewrite. I'm not sure that the overhead of checking the units is going to be that much different from the overhead of rewriting with conversion factors. Both processes have to traverse the entire structure after all. Why do that twice? The check_unit approach already has all the information it needs to correct the equations, so it makes sense to me to just go ahead and correct it.

One place where this is really suboptimal: symbolic array equations. Those could be unit-checked once for the entire array equation, in principle, but right now the equations get scalarized before they arrive so the checking is needlessly repeated.

[lamorton]

Closing because stale.