Skip to content

fix seed-reset order for no-op cases where input === the output#204

Merged
jrevels merged 1 commit intomasterfrom
jr/seedorderfix
Mar 16, 2017
Merged

fix seed-reset order for no-op cases where input === the output#204
jrevels merged 1 commit intomasterfrom
jr/seedorderfix

Conversation

@jrevels
Copy link
Member

@jrevels jrevels commented Mar 15, 2017

See JuliaDiff/ReverseDiff.jl#60.

Before this PR:

julia> ForwardDiff.jacobian(identity, rand(11))
11×11 Array{Float64,2}:
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  1.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  1.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  1.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  1.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  1.0

After this PR:

julia> ForwardDiff.jacobian(identity, rand(11))
11×11 Array{Float64,2}:
 1.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  1.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  1.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  1.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  1.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  1.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  1.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  1.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  1.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  1.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  1.0

Test added in DiffBase here. Note tests will fail once JuliaAttic/DiffBase.jl#3 is merged and tagged, because ForwardDiff is testing against Calculus and Calculus appears to be getting this wrong as well:

julia> Calculus.jacobian(identity, rand(11), :forward)
11×11 Array{Float64,2}:
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0
 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0

@jrevels jrevels mentioned this pull request Mar 15, 2017
Merged
@jrevels jrevels merged commit 6c61b61 into master Mar 16, 2017
@jrevels jrevels deleted the jr/seedorderfix branch March 16, 2017 21:17
tkelman pushed a commit to tkelman/ForwardDiff.jl that referenced this pull request Mar 17, 2017
@jrevels @tkelman
fix seed-reset order for no-op cases where input === the output (Juli…
5e9cecc 
…aDiff#204)

(cherry picked from commit 6c61b61)
jrevels pushed a commit that referenced this pull request Mar 21, 2017
@tkelman @jrevels
Cherry-pick #204 to a new release-0.3 branch that supports Julia 0.4 (#…
805ad5d 
…205)

* Revert "bump version requirements for Calculus, DiffBase, and NaNMath to include v0.6 fixes (#184)"

This reverts commit 62ad04b.
If higher minimum versions are only needed for 0.6, they should be loosened here

Upper bound this branch so it is not installable on 0.6.

* fix seed-reset order for no-op cases where input === the output (#204)

(cherry picked from commit 6c61b61)

* don't test this branch on nightly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jrevels

Comments