Closed jmert closed 4 years ago
Failures on v1.5 and master are due to a change in how LLVM is optimizing the coeff_α
and coeff_β
functions for the spherical normalization case. The issue has been unnoticed thus far because there were no written/tested examples of particular affected numerical values until the rewrite of the documentation introduced in this PR.
I've asked a generic question on Julia's Discourse to see if there's more information about what LLVM is doing specifically, but in the mean time I'll add an appropriate workaround commit to bring the calculation back into alignment on all Julia versions.
Merging #19 into master will increase coverage by
0.27%
. The diff coverage is97.87%
.
@@ Coverage Diff @@
## master #19 +/- ##
==========================================
+ Coverage 95.52% 95.80% +0.27%
==========================================
Files 9 9
Lines 246 262 +16
==========================================
+ Hits 235 251 +16
Misses 11 11
Impacted Files | Coverage Δ | |
---|---|---|
src/calculation.jl | 96.15% <95.65%> (+0.42%) |
:arrow_up: |
src/broadcasting.jl | 100.00% <100.00%> (ø) |
|
src/norm_sphere.jl | 100.00% <100.00%> (ø) |
|
src/precompile.jl | 100.00% <100.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 3bc3355...fac3b3d. Read the comment docs.
This does a large overhaul of how array arguments are handled, namely making
legendre()
a vector function and "downgrading" the status/importance of broadcasting.Highlights:
legendre(norm, 0:lmax, 0:mmax, x)
will now allocate and return an appropriately-shaped output for calculating all degrees/orders for all x (from scalar through any-dimensional array). This effectively fixes #8.legendre()
now does array-based operations, the advertisement of broadcastedlegendre.()
has been removed from the manual. There are stillbroadcasted(::typeof(legendre))
specializations, though, to intercept any accidental broadcasting that would be better served by a single vectorized invocation._legendre!
function tounsafe_legendre!
, and moved around argument checking so that it's truly unsafe and no need for@propagate_inbounds
(so should save time/effort in compiling multiple long inlining of the full implementation).