Fix seeding with undefined elements #743
Conversation
|
I didn't know about |
I put it behind an |
| end | ||
| end | ||
|
|
||
| @testset "BigFloat" begin |
There was a problem hiding this comment.
Does this test cover all the new branches? It seems CodeCov is sleeping at the wheel.
I think we may also want to add cases where only some of the values are uninitialized?
There was a problem hiding this comment.
Codecov results are available on the codecov webpage: https://app.codecov.io/github/JuliaDiff/ForwardDiff.jl/commit/6f8f6a977d4d69fda7c492f6e0a47cb8a6853045 (I assume the codecov app is not installed in the repo and hence there's no nice summary in the PR - AFAICT I don't have sufficient permissions to install it)
Indeed, you were right, the tests did not cover all new branches. I extended the tests, and locally it seems that they cover all branches as commenting out any of the branches leads to test failures.
There was a problem hiding this comment.
Codecov confirms that all modified lines are covered by the tests: https://app.codecov.io/github/JuliaDiff/ForwardDiff.jl/commit/ec3e483747159b46e0db5328c60c3543c13478a6/indirect-changes
| if isassigned(x, idx) | ||
| duals[idx] = Dual{T,V,N}(x[idx], seed) | ||
| else | ||
| Base._unsetindex!(duals, idx) |
There was a problem hiding this comment.
:/, this is quite unfortunate, is this really the only way to fix this?
There was a problem hiding this comment.
I think the idea is that if x[idx] is #undef, we want duals[idx] to be the same
There was a problem hiding this comment.
Yeah, I wasn't 100% percent happy but it was the best I could think of and what apparently is used in base. IMO we really want to ensure here that we get the same behaviour as copyto! on the primal part, to avoid that yduals contains any surprising values, possibly from previous chunks or differentations.
There was a problem hiding this comment.
julia> x = BigFloat[1, 2];
julia> y = similar(x); y[2] = 1;
julia> x
2-element Vector{BigFloat}:
1.0
2.0
julia> y
2-element Vector{BigFloat}:
#undef
1.0
julia> copyto!(x, y);
julia> x
2-element Vector{BigFloat}:
#undef
1.0
|
What's the word on this one? Do we want it? |
|
gentle bump from me as well |
|
Bump again |
|
I guess let's go with |
|
I opened JuliaLang/julia#59394 regarding |
Fixes #436. Fixes #740.