Skip to content
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

zero on uninitialized arrays now fail in some cases #53582

Closed
KristofferC opened this issue Mar 4, 2024 · 4 comments · Fixed by #53602
Closed

zero on uninitialized arrays now fail in some cases #53582

KristofferC opened this issue Mar 4, 2024 · 4 comments · Fixed by #53602
Labels
regression Regression in behavior compared to a previous version
Milestone

Comments

@KristofferC
Copy link
Member

KristofferC commented Mar 4, 2024

From https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/0520b80_vs_997b49f/Unitful.primary.log.

Expression: zero(Vector{Union{Quantity{Float64, 𝐋}, Missing}}(undef, 1)) == [0.0m]
  UndefRefError: access to undefined reference

Looking a bit at the blame I think this was introduced in #51458.

cc @oxinabox

@KristofferC KristofferC added the regression Regression in behavior compared to a previous version label Mar 4, 2024
@KristofferC KristofferC added this to the 1.11 milestone Mar 4, 2024
@KristofferC
Copy link
Member Author

I think this also changed the following


# 1.10
julia> zero(Union{typeof(0.0s), Missing}[missing])
1-element Vector{Quantity{Float64, 𝐓, Unitful.FreeUnits{(s,), 𝐓, nothing}}}:
 0.0 s

# 1.11
julia> zero(Union{typeof(0.0s), Missing}[missing])
1-element Vector{Missing}:
 missing

@oxinabox
Copy link
Contributor

oxinabox commented Mar 5, 2024

Interesting. This certainly wasn't intended
It also effects even just Base

julia>  zero(Union{typeof(0.0), Missing}[missing])
1-element Vector{Missing}:
 missing

If we can't resolve this quickly, I think best is to revert #51458.
so as not to hold up the release, and then think about it a bit harder, for the 1.12 release

There actually is code in unitful to special case this, but it is excluded after v1.8.
So presumably there was a PR to julia that fixed that so it wasn't needed.
https://github.com/PainterQubits/Unitful.jl/blob/a09bdaac255d456e6315b77979a48956aca1d3a3/src/quantities.jl#L384-L390

@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Mar 8, 2024
@KristofferC
Copy link
Member Author

Shouldn't have been closed #53602 (comment)

@KristofferC KristofferC reopened this Mar 9, 2024
@JeffBezanson JeffBezanson removed the merge me PR is reviewed. Merge when all tests are passing label Mar 11, 2024
@KristofferC
Copy link
Member Author

This seems to pass on beta now... Not really sure what fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Regression in behavior compared to a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants