JuliaDiff / DiffRules.jl

A simple shared suite of common derivative definitions

Other

75 stars 38 forks source link

Add `iszero(x)` branches to `xlogy` and `xlog1py` #85

Closed simsurace closed 2 years ago

simsurace commented 2 years ago

It is unclear to me how the tests work here. I did not find the tests of corner cases.

codecov-commenter commented 2 years ago

Codecov Report

Merging #85 (d324416) into master (5a121b8) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #85   +/-   ##
=======================================
  Coverage   97.00%   97.00%           
=======================================
  Files           3        3           
  Lines         167      167           
=======================================
  Hits          162      162           
  Misses          5        5

Impacted Files	Coverage Δ
src/rules.jl	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

simsurace commented 2 years ago

The failures in Nabla.jl and EAGO.jl seem unrelated to this PR.

simsurace commented 2 years ago

Is this fine to be merged?

simsurace commented 2 years ago

I'm bumping this as it is still unclear to me what if anything is missing.

simsurace commented 2 years ago

Then I think we can merge and I'll change https://github.com/JuliaStats/LogExpFunctions.jl/pull/54 to follow the same guidelines.

simsurace commented 2 years ago

This should be fine to be merged now as well.

simsurace commented 2 years ago

Sure. I just don't know where to put them, since there currently do not seem to be any tests for xlogy and xlog1py. Or where should I look for them?

EDIT: or should new branches be created under the arity == 2 branch?

devmotion commented 2 years ago

Sure. I just don't know where to put them, since there currently do not seem to be any tests for xlogy and xlog1py.

All rules apart from the ones excluded in https://github.com/JuliaDiff/DiffRules.jl/blob/2c29d7d59c09f1ca078192fa24eef8d7fba7738e/test/runtests.jl#L17 are tested in https://github.com/JuliaDiff/DiffRules.jl/blob/2c29d7d59c09f1ca078192fa24eef8d7fba7738e/test/runtests.jl#L19-L110.

Tests of such special cases might be best to add below https://github.com/JuliaDiff/DiffRules.jl/blob/2c29d7d59c09f1ca078192fa24eef8d7fba7738e/test/runtests.jl#L142, i.e., below the special tests for rem2pi and ldexp.