-
Notifications
You must be signed in to change notification settings - Fork 149
Fix seeding with undefined elements #743
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
I didn't know about |
I put it behind an |
@@ -279,4 +279,18 @@ end | |||
end | |||
end | |||
|
|||
@testset "BigFloat" begin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:/, this is quite unfortunate, is this really the only way to fix this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Fixes #436. Fixes #740.