StackOverflowError from @fastmath

[ChrisRackauckas]

Unitful.jl is incompatible with @fastmath. Here's a small example showing how to get the StackOverflowError (requires that you have optimized Julia to your system, like re-built the system image, and I can only get this error on Linux but not Windows)

using Unitful
u = 1.5u"N"
halfΔt = 0.03125u"s"
tmp2 = .25u"N/s"
u+halfΔt*tmp2
@fastmath u+halfΔt*tmp2

SIUnits has the same error at Keno/SIUnits.jl#94

[ajkeller34]

Thanks for filing this issue. I can reproduce this on the release binary of 0.5.0 for macOS Sierra. The problem seems to be that promotion doesn't always return the same type if there are quantities involved (because it can't, there's no safe type to promote to from quantities with units N/s and s, which have different dimensions). So you get a StackOverflowError at line 241 of fastmath.jl in base. It's reasonable that fastmath.jl expects promotion to always return the same types, which is true for numeric types in Base, but promotion is not required to return the same types, e.g. promote("a", 1) == ("a",1).

I don't think it would be that hard to work around by just specializing some methods for Quantities, hopefully I'll get some time to do this soon. I'm more concerned about the following behavior:

julia> promote(1u"s", 1.0u"s")
(1.0 s,1.0 s)

julia> promote(1u"s", 1.0u"N/s")
(1 s,1.0 N s^-1)

I consider this a bug in Unitful---promotion should at least make the "numeric part" of the types the same, in my view.

[ajkeller34]

Having implemented some important changes to promotion, I now have @fastmath working nicely on my local repository; once I finish repeating most of the fastmath tests from base (but with units) I will push to master. I guess that will be in another day or two. Making sure I haven't introduced performance regressions is another matter and I would be curious to see if you notice any. Also, fma is implemented on master already if you make use of that.

[ChrisRackauckas]

This sounds great. Is there any way these promotions could be also added to SIUnits so it's compatible? I'd hate to have to drop SIUnits completely, but it is looking more and more like a "legacy" library.

[ajkeller34]

Fixed on master.

Regarding SIUnits, I think it already has the correct behavior for promotion needed to get fastmath working (promoting numbers and quantities will turn the number into a unitless quantity, and the numeric backing types are promoted). Most of what I've done in this package could probably be ported over without much trouble. I don't have time to do that myself, but please feel free to try it out based on my code and submit a PR there if needed. It's mostly just small tweaks to what is found in fastmath.jl in base.

[ajkeller34]

*note that you may need to delete deps/Defaults.jl and rerun Pkg.build("Unitful") after updating

[ChrisRackauckas]

Was going to give this a try on master but got a precompilation error:

ERROR: LoadError: LoadError: UndefVarError: DimensionedUnits not defined
 in include_from_node1(::String) at .\loading.jl:532
 in include(::String) at .\sysimg.jl:14
 in defaults() at C:\Users\Chris\.julia\v0.6\Unitful\src\User.jl:261
 in include_from_node1(::String) at .\loading.jl:532
 in include(::String) at .\sysimg.jl:14
 in macro expansion; at .\none:2 [inlined]
 in anonymous at .\<missing>:?
 in eval(::Module, ::Any) at .\boot.jl:238
 in process_options(::Base.JLOptions) at .\client.jl:245
 in _start() at .\client.jl:332
while loading C:\Users\Chris\.julia\v0.6\Unitful\deps\Defaults.jl, in expression starting on line 141
while loading C:\Users\Chris\.julia\v0.6\Unitful\src\Unitful.jl, in expression starting on line 878

[ajkeller34]

Did you try deleting deps/Defaults.jl in the Unitful package directory and rerunning Pkg.build("Unitful")? I hope not to break the defaults file too often but it was necessary in the most recent update. If you have ideas for how I can warn the user more gracefully when the defaults have to change, please provide feedback at #33.

[ChrisRackauckas]

Works beautifully. Thanks