rafaqz / UnitfulMoles.jl

Utilities for working with Mole units for the Julia language
Other
15 stars 2 forks source link

Problem in 0.1.1 #19

Closed AlexisRenchon closed 2 years ago

AlexisRenchon commented 2 years ago

0.1.1 works locally for me:

julia> include("DAMMmodel.jl")
Main.DAMMmodel

julia> using Main.DAMMmodel

julia> Tₛ = [18.0, 22.0] # soil temperature [°C]
2-element Vector{Float64}:
 18.0
 22.0

julia> θ = [0.35, 0.22] # soil moisture [m³ m⁻³]
2-element Vector{Float64}:
 0.35
 0.22

julia> x = hcat(Tₛ, θ)
2×2 Matrix{Float64}:
 18.0  0.35
 22.0  0.22

julia> p = (1e9, 64.0, 3.46e-8, 2.0e-3, 0.4, 0.0125) # αₛₓ, Eaₛₓ, kMₛₓ, kMₒ₂, Sxₜₒₜ
(1.0e9, 64.0, 3.46e-8, 0.002, 0.4, 0.0125)

julia> DAMM(x, p)
2-element Vector{Float64}:
 1.585878871033841
 2.809139397295634

But it doesn't work when registered

julia> using DAMMmodel

julia> Tₛ = [18.0, 22.0] # soil temperature [°C]
2-element Vector{Float64}:
 18.0
 22.0

julia> θ = [0.35, 0.22] # soil moisture [m³ m⁻³]
2-element Vector{Float64}:
 0.35
 0.22

julia> x = hcat(Tₛ, θ)
2×2 Matrix{Float64}:
 18.0  0.35
 22.0  0.22

julia> p = (1e9, 64.0, 3.46e-8, 2.0e-3, 0.4, 0.0125) # αₛₓ, Eaₛₓ, kMₛₓ, kMₒ₂, Sxₜₒₜ
(1.0e9, 64.0, 3.46e-8, 0.002, 0.4, 0.0125)

julia> DAMM(x, p)
ERROR: KeyError: key Symbol("Moles CO₂") not found
Stacktrace:
  [1] getindex(h::Dict{Symbol, Tuple{Float64, Rational{Int64}}}, key::Symbol)
    @ Base ./dict.jl:481
  [2] basefactor
    @ ~/.julia/packages/Unitful/woO6b/src/units.jl:258 [inlined]
  [3] map
    @ ./tuple.jl:223 [inlined]
  [4] basefactor(x::Unitful.FreeUnits{(m^-2, μmolCO₂, s^-1), 𝐌 𝐋^-2 𝐓^-1, nothing})
    @ Unitful ~/.julia/packages/Unitful/woO6b/src/units.jl:261
  [5] #s81#159
    @ ~/.julia/packages/Unitful/woO6b/src/conversion.jl:19 [inlined]
  [6] var"#s81#159"(::Any, s::Any, t::Any)
    @ Unitful ./none:0
  [7] (::Core.GeneratedFunctionStub)(::Any, ::Vararg{Any})
    @ Core ./boot.jl:580
  [8] uconvert(a::Unitful.FreeUnits{(m^-2, μmolCO₂, s^-1), 𝐌 𝐋^-2 𝐓^-1, nothing}, x::Unitful.Quantity{Float64, 𝐌 𝐋^-2 𝐓^-1, Unitful.FreeUnits{(m^-2, molC, s^-1), 𝐌 𝐋^-2 𝐓^-1, nothing}})
    @ Unitful ~/.julia/packages/Unitful/woO6b/src/conversion.jl:78
  [9] FreeUnits
    @ ~/.julia/packages/Unitful/woO6b/src/types.jl:99 [inlined]
 [10] |>
    @ ./operators.jl:966 [inlined]
 [11] _broadcast_getindex_evalf
    @ ./broadcast.jl:670 [inlined]
 [12] _broadcast_getindex
    @ ./broadcast.jl:643 [inlined]
 [13] getindex
    @ ./broadcast.jl:597 [inlined]
 [14] copy
    @ ./broadcast.jl:899 [inlined]
 [15] materialize
    @ ./broadcast.jl:860 [inlined]
 [16] DAMM(x::Matrix{Float64}, p::NTuple{6, Float64})
    @ DAMMmodel ~/.julia/packages/DAMMmodel/MXkov/src/functions/maths/DAMM.jl:44
 [17] top-level scope
    @ REPL[7]:1

julia> molCO₂
molCO₂

julia> typeof(molCO₂)
Unitful.FreeUnits{(molCO₂,), 𝐌, nothing}

I am not sure what is happening

AlexisRenchon commented 2 years ago

Don't bother about it yet though, I'm going to try a few things to fix it within DAMMmodel, I'll keep you updated

rafaqz commented 2 years ago

You should never do include of a package, using instead. Otherwise Im not sure.

AlexisRenchon commented 2 years ago

I did include to to use my Module locally, what is the right way? like this?

] activate . 
using DAMMmodel

This gave me the error

ERROR: KeyError: key Symbol("Moles CO₂") not found

as well.

my Module look like this:

module DAMMmodel

using Statistics
using LsqFit
using DataFrames
using SparseArrays
using UnicodeFun
using GLMakie
using Unitful: R, L, mol, K, kJ, °C, m, g, cm, hr, mg, s, μmol
using UnitfulMoles: molC
using Unitful, UnitfulMoles
@compound CO₂

include(joinpath("constructors", "constants.jl"))
include(joinpath("constructors", "constructors.jl"))
include(joinpath("functions", "maths", "DAMM.jl"))
include(joinpath("functions", "maths", "DAMMfit.jl"))
include(joinpath("functions", "maths", "DAMMmat.jl"))
include(joinpath("functions", "viz", "DAMMplot.jl"))
include(joinpath("functions", "viz", "DAMMviz.jl"))
include(joinpath("functions", "maths", "qbins.jl"))
export DAMM, DAMMfit, DAMMmat, DAMMplot, DAMMviz, qbins, sDAMMmat, sDAMMmatq

end

I tried to add molCO2to the export list, but it didn't help

if I run the Module (with include or just running all lines) then do using DAMMmodel, everything works... it seems that @compound doesn't work when inside a package only, and I have no idea why...

if I do

using Unitful, UnitfulMoles
@compound CO2

after loading the package, then it works.

do you know of any other package using @compound? If yes, maybe they implement it differently and I could fix my issue doing it similarly?

I also tried to put @compound CO2 inside the function that uses it, but that creates a bug.

AlexisRenchon commented 2 years ago

According to @briochemc : It's probable that the @compound macro was only intended for REPL use and that it does not follow the instructions in https://painterqubits.github.io/Unitful.jl/stable/extending/#Extending-Unitful

rafaqz commented 2 years ago

Well seeing I wrote it... I was trying to make it work everywhere I just didn't know how at the time. I was really new to Julia and this is a pretty horrific level of nested macros.

But it absolutely can work anywhere, and I tested it with your package and using, but with your latest changes it doesn't work. Maybe we can find out why.

You can use @compound or use @unit (as it does) inside a function, because you cant define types inside a function - thats just how julia works.

AlexisRenchon commented 2 years ago

Do you have the version of my package that worked? - was probably 0.1.7

Did you change anything in the module when you tried it?

I didn't change anything basically... I did mess around with the module DAMMmodel to try to make it work, but I didn't change the function that uses mol CO2.

I am not sure what you did when you say "and I tested it with your package and using" but it sounds like you made it work and it should still work

rafaqz commented 2 years ago

I didn't merge that PR without your package working, that was the point ;)

(But actually this problem is from more detailed use that I didn't check, now I look at it)

rafaqz commented 2 years ago

I dont know what version now because I devved it and just pulled your update.

rafaqz commented 2 years ago

Here is I think what the problem is - specifically with gram conversion:

using DAMMmodel, Unitful

uconvert(u"g", 2DAMMmodel.molCO₂)

Seems like a small bug in @compound

AlexisRenchon commented 2 years ago

The only thing that might be weird in my package in the module in DAMMmodel.jl It was my bad that I didn't know how to test it properly... but the good thing in that I learned something, always do ] activate . to test locally, not include()

AlexisRenchon commented 2 years ago

the function using Unitful is here https://github.com/CUPofTEAproject/DAMMmodel.jl/blob/master/src/functions/maths/DAMM.jl

and the lines 36-38 where I use UnitfulMoles stuff:

  Rₛₘₛ = @. Rₛₘ * Eₛ # Respiration, effective depth 10 cm, mg C cm⁻² hr⁻¹ 
  Rₛₛₘ = Rₛₘₛ .|> molC*m^-2*s^-1 
  Rₛ = Rₛₛₘ .|> μmolCO₂*m^-2*s^-1

(It works fine except when @compound CO2 is inside the module as I put it in https://github.com/CUPofTEAproject/DAMMmodel.jl/blob/master/src/DAMMmodel.jl)

rafaqz commented 2 years ago

Its hard to test because just including this module doesn't make the error:

module TestModule
    using Unitful, UnitfulMoles
    @compound CO2
end

uconvert(u"g", 1TestModule.molCO₂)

It has to be a package...

AlexisRenchon commented 2 years ago

do you just run the module? or do you do ] activate .? everything works for me when I run the module in Julia REPL, it only bugs if I do ] activate .

AlexisRenchon commented 2 years ago
(@v1.7) pkg> activate .
  Activating project at `~/MyPackages/DAMMmodel`

julia> using DAMMmodel
[ Info: Precompiling DAMMmodel [7af7f183-cd03-4039-bf6b-67ea3eb1648d]

julia> DAMM([18.2 0.3], (1e9, 64.0, 3.46e-8, 2.0e-3, 0.4, 0.0125))
ERROR: KeyError: key Symbol("Moles CO₂") not found
julia> include("DAMMmodel.jl")
Main.DAMMmodel

julia> using Main.DAMMmodel

julia> DAMM([18.2 0.3], (1e9, 64.0, 3.46e-8, 2.0e-3, 0.4, 0.0125))
1-element Vector{Float64}:
 1.8836935508519232
rafaqz commented 2 years ago

Ok I think the problem is you have to register your package with Unitful.jl for all the macros to work with a package.

UnitfulMoles.jl has this code, it doesn' totally make sense to me but maybe you can swap in DAMMmodel for UnitfulMoles.

const localunits = Unitful.basefactors
function __init__()
    merge!(Unitful.basefactors, localunits)
    Unitful.register(UnitfulMoles)
end
AlexisRenchon commented 2 years ago

ok I'll try to add that in my module, 2 min

rafaqz commented 2 years ago

Hmm doesn't seem to work with UnitfulMoles defining compounds.

At least I know what the problem is now - I'll define a test compound in UnitfulMoles to be sure it works doing it in a package.

AlexisRenchon commented 2 years ago

it did work for me!

(@v1.7) pkg> activate .
  Activating project at `~/MyPackages/DAMMmodel`

julia> using DAMMmodel
[ Info: Precompiling DAMMmodel [7af7f183-cd03-4039-bf6b-67ea3eb1648d]

julia> DAMM([18.2 0.3], (1e9, 64.0, 3.46e-8, 2.0e-3, 0.4, 0.0125))
1-element Vector{Float64}:
 1.8836935508519232
rafaqz commented 2 years ago

Sweet! Thats great. We should add this to the readme so its clear you need to register with unitful for this to work from inside a package.

AlexisRenchon commented 2 years ago

wait before closing, see if it works when I register

rafaqz commented 2 years ago

Ok. I've added this to the README, thanks for your patience working through this!

AlexisRenchon commented 2 years ago

Thank you for helping me! I really like what Unitful and UnitfulMoles bring to work with units. I've never done something like that in programming before. I hope CliMA ESM will use it and will allow using DAMMmodel as a module for their LSM.

rafaqz commented 2 years ago

Yeah it's pretty wild that this actually works. But do make sure to test your results independently sometimes ;)

I don't really work with mols of things anymore so mostly just use Unitful.jl, but yes its a huge game changer for physical/mechanistic models.

rafaqz commented 2 years ago

I think just this will work for you:

function __init__()
    Unitful.register(DAMMmodel)
end
AlexisRenchon commented 2 years ago

ok the registered package worked, we can close

AlexisRenchon commented 2 years ago

I will test

function __init__()
    Unitful.register(DAMMmodel)
end
AlexisRenchon commented 2 years ago

yes, just

function __init__()
    Unitful.register(DAMMmodel)
end

works