Skip to content

Conversation

oxinabox
Copy link
Member

Fixes JuliaMath/SpecialFunctions.jl#288 (comment)

This is being reported now because of #84
which added checking that the primal type when it was checking if two Composite{Primal} were equal.
but looking at it the code for determining the primal type if the Composite came from FiniteDifferencing is wrong

_maybe_fix_to_composite(x::Tuple) = Composite{typeof(x)}(x...)
_maybe_fix_to_composite(x::NamedTuple) = Composite{typeof(x)}(;x...)
_maybe_fix_to_composite(x) = x

because it just assumes that that type of the difference is the type of the primal.
Which it is not for the case for Integers.

That code was added in #89 and #87 That PR was not very successful, since this the third bug fix i will need to make to it.

Copy link
Member

@simeonschaub simeonschaub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small style nitpick, otherwise LGTM.

Co-authored-by: Simeon Schaub <simeondavidschaub99@gmail.com>
@oxinabox oxinabox merged commit 60dcb1c into master Dec 21, 2020
oxinabox added a commit that referenced this pull request Dec 21, 2020
* Get primal type from primal when fixing composites

* removed unused type alias

* Update src/testers.jl

Co-authored-by: Simeon Schaub <simeondavidschaub99@gmail.com>

Co-authored-by: Simeon Schaub <simeondavidschaub99@gmail.com>
@oxinabox oxinabox mentioned this pull request Dec 21, 2020
oxinabox added a commit that referenced this pull request Dec 21, 2020
* Get primal type from primal when fixing composites (#93)

* Get primal type from primal when fixing composites

* removed unused type alias

* Update src/testers.jl

Co-authored-by: Simeon Schaub <simeondavidschaub99@gmail.com>

Co-authored-by: Simeon Schaub <simeondavidschaub99@gmail.com>

* bump version

Co-authored-by: Simeon Schaub <simeondavidschaub99@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test failure with ChainRulesTestUtils on Julia 1.3

2 participants