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

Initialization problems with preferunits for physical constants on Julia 1.8 #545

Open
j-fu opened this issue Jul 20, 2022 · 6 comments
Open

Comments

@j-fu
Copy link

j-fu commented Jul 20, 2022

Hi, I struggle with a subtle issue concerning the definition of physical constants in unitful.jl after preferunits has been set.

I set up a small package on https://github.com/j-fu/UTest.jl which can be dev'd for testing.

In the root directory it contains utest.jl which can be called either with

julia --project=. --compile=min utest.jl`

or with

julia --project=. utest.jl`

The correct output is

Unitful.upreferred(1 * u"m") = 100 cm
ustrip(upreferred(u"ε0")) = 8.85418781762039e-18
ustrip(upreferred(Unitful.ε0)) = 8.85418781762039e-18
ustrip(upreferred(PhysicalConstants.CODATA2018.ε_0)) = 8.8541878128e-18

This is obtained on all older Julia versions, and on 1.8 (beta, rc3) with --compile=min

On 1.8 (beta, rc3) and 1.9 (f1991edd30) without --compile=min one gets

Unitful.upreferred(1 * u"m") = 100 cm
ustrip(upreferred(u"ε0")) = 8.85418781762039e-12
ustrip(upreferred(Unitful.ε0)) = 8.85418781762039e-12
ustrip(upreferred(PhysicalConstants.CODATA2018.ε_0)) = 8.8541878128e-18

One gets the correct result with optimized code if after modification, the --compile=min has been called first. The problem occurs only for physical constants, not for units. PhysicalConstants.jl behaves in the right way, though.

I see three possibilities here:

  • Problem with the architecture of Unitful
  • Problem with code loading in Julia 1.8 when optimizing
  • Evaluating upreferred into the global context in a package using Unitful is a bad idea, even if preferunits is called before loading the package. If this is the case, it needs to be documented. (in fact I struggled with this situation in more obvious situations, so I have an Idea what to write into a PR...)
j-fu added a commit to j-fu/LessUnitful.jl that referenced this issue Jul 20, 2022
Now we have two sets of Physical constants accessible this way:
 Na,q,k etc. from Unitful (didn't see them before)
 N_A,e, k_B from PhysicalConstants.CODATA2018

The former are buggy under Julia 1.8, see
PainterQubits/Unitful.jl#545

Also, this taps now into the deeper mechanisms of Unitful. Look like a rabbit hole...
@j-fu
Copy link
Author

j-fu commented Jul 22, 2022

May be also linked to JuliaLang/julia#45861

Checked: it is not in 1.8-rc3 but has been added to the 1.8 backports so we may expect it in one of the next rc's.

Edit: in nighly f1991edd30 the problem persists.

@j-fu
Copy link
Author

j-fu commented Aug 14, 2022

Update: the problem persists in 1.8.0-rc4

@sostock
Copy link
Collaborator

sostock commented Jan 5, 2023

Now that I’ve looked at this, I’m surprised that it ever worked. The problem is here:

Unitful.jl/src/units.jl

Lines 304 to 307 in dc044ee

@generated function upreferred(x::Dimensions{D}) where {D}
u = prod((NoUnits, (promotion[name(z)]^z.power for z in D)...))
:($u)
end

This is a generated function that depends on non-constant global state (Unitful.promotion), which is not allowed.

Unfortunately, just making this function not @generated will make every promotion operation horribly slow, because it would involve looking up the units in the Unitful.promotion dictionary every time.

I see two possible solutions:

  1. Remove the possibility to set a global unit preference via preferunits. This would of course be breaking. I have opened an issue to discuss this: preferunits without global state #601
  2. Add some code to preferunits so that every call to it deletes the upreferred(x::Dimensions{D}) where {D} method and then defines it again. I haven’t tried whether that would actually work. For example, other generated functions that call upreferred might not get recompiled. There might be issues with precompilation as well.

@j-fu
Copy link
Author

j-fu commented Jan 5, 2023

Wow this seems to be a tough one - thanks for consideration!

@timholy
Copy link
Contributor

timholy commented Jan 5, 2023

We don't keep backedges to the generator, so if it's called from the generator then indeed nothing will be recompiled.

@vtjnash
Copy link
Contributor

vtjnash commented Jan 6, 2023

promotion[name(z)] is mutable, and therefore should not be accessed from a generator. It should be written as something dispatch-compatible, such as get_promotion(Val(name(z)), where get_promotion(::Val{:Q}) = promotion[:Q]

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

No branches or pull requests

4 participants