-
Notifications
You must be signed in to change notification settings - Fork 421
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
Unitful support? #1413
Comments
I am wondering the same. Replacing some > using Distributions
> using Unitful: mV, Hz, s
> n = Normal(0mV, 1mV)
Normal{Unitful.Quantity{…}}(μ=0 mV, σ=1 mV)
> pdf(n, 3mV)
0.0044318484119380075 mV^-1
> λ = 4Hz;
> e = Exponential(1/λ)
Exponential{Unitful.Quantity{Float64, 𝐓, Unitful.FreeUnits{(Hz^-1,), 𝐓, nothing}}} (θ=0.25 Hz^-1)
> rand(e) |> s
0.6119753615525725 s How fun! Of course more patches are needed. |
Just replacing |
Not in general but only for distributions without atoms - ie. eg. not for Thus it seems supporting the limit case (Clearly, |
It's unfortunate we use |
Another - but probably non-standard - approach would be to choose different base measures (they're all implicit anyway currently) that ensure that |
It's a density in the discrete case as well, just with respect to the counting measure. So the name |
+1 that unitful support would be cool ^_^ |
I gave this a go a while back: master...tfiers:Distributions.jl:master I implemented unit support for Changes done to make this work:
I made sure that there was no change in current usage of Distributions.jl. I started a docs page with a list of "Supported distributions" (just the two), and a guide for how to add unit support to a new distribution. |
|
Therefore, to minimize the impact of unit support in Distributions.jl on the 99% of users that don't need it, there first needs to be created a lightweight package defining just the interface/API for working with units ( Instead of importing the full Unitful package, Distributions will then instead import just the interface package, with minimal impact on load time. |
An update, from the discussion at with Julia 1.9's 'extension packages' (https://pkgdocs.julialang.org/dev/creating-packages/#Conditional-loading-of-code-in-packages-(Extensions)), a unitful core or interface package is no longer needed, and the full Though, given that is 1.9+ only, I'm not sure how this interacts with Distributions.jl's current support for Julias down to 1.0 UPDATE: See this helpful summary by mbauman and chrisr that anwers these questions: https://discourse.julialang.org/t/package-extensions-for-julia-1-9/93397/5?u=tfiers |
Is there a consensus on how to address this issue? I am seeing open PRs for Exponential, but that is it. What would be the minimum amount of work that one would need to put in a PR to get this feature gradually added? Julia has package extensions nowadays, so the Unitful.jl load time is no longer an issue. |
No, there isn't. IMO the problem is more a conceptual one and the fact that it would be require changes everywhere since otherwise you end up in a messy half-baked situation as has been the case for multiple years after moving from hardcoded Float64 to Real. There are also more pressing items that also involve changes throughout Distributions, in particular since it is just a convenient feature but not something that's absolutely necessary - you can just perform calculations after stripping units. |
Isn't a half-baked situation better than never-baked situation in this case? What is the problem with releasing patches whenever a new distribution is generalized to Number? Are you concerned that some distributions may not be well-defined with units at all? |
Maybe some distributions might be problematic. The more common problem though, I assume, is that many parameters that currently have the same type I've experienced this Float64->Real transition, and there are still problems to fix even though that happened a long time ago. Since IMO it's really just a convenience thing, I'm strongly against a half-baked solution. There's just too many other things we could and should spend time on rather than cleaning up support for unitful numbers IMO. |
Using
Unitful v1.9.1
andDistributions v0.25.23
, it appearsUnitful
numbers are not supported:Could this be supported without undue pain?
The text was updated successfully, but these errors were encountered: