simsurace commented 2 years ago

This is supposed to fix https://github.com/JuliaStats/LogExpFunctions.jl/issues/44.

There may be more optimal orders of operations for these derivatives. Also, could someone assist me in what would be mandatory tests for these additions?

simsurace commented 2 years ago

Looks like https://github.com/JuliaStats/LogExpFunctions.jl/pull/45 is actually a prerequisite.

devmotion commented 2 years ago

It's already tested automatically but maybe it's helpful to adjust the domain of inputs (both for the primal and the finite diff comparisons), similar to some other functions in runtests.jl. I don't remember the default domain by heart, so maybe it's not needed at all.

Ironically, tests fail because the functions are not defined for Float32 (yet). So for the time being, probably we should skip Float32 tests for these inputs (or check that they throw an error, to be notified when this changes).

codecov-commenter commented 2 years ago

Codecov Report

Merging #82 (b1795d3) into master (7dde134) will increase coverage by 0.03%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #82      +/-   ##
==========================================
+ Coverage   96.96%   97.00%   +0.03%     
==========================================
  Files           3        3              
  Lines         165      167       +2     
==========================================
+ Hits          160      162       +2     
  Misses          5        5

Impacted Files	Coverage Δ
src/rules.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 7dde134...b1795d3. Read the comment docs.

devmotion commented 2 years ago

The MTK test errors are unrelated.

JuliaDiff / DiffRules.jl

Add diffrules for `log1pmx` and `logmxp1` #82

Codecov Report