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

Introduce a Unitful extension #126

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Ickaser
Copy link

@Ickaser Ickaser commented Feb 14, 2025

Hello,

I was perusing documentation for Turing and DynamicHMC recently, and realized that in quite a lot of my code I have been implementing my own transforms, in the fragile way where things are hard-coded everywhere. I think I could stand to clean up and standardize a lot of that code just by having transforms like in this package.

Rather than attempt to broaden the number types of e.g. ShiftedExp to admit Unitful quantities, this PR adds a dimensional DimShiftedExp, which takes advantage of the type signatures of Unitful quantities to encode the destination units in the type itself.

Some things that this PR makes possible:

trans = as(Real, 0.0u"s",  20u"hr") # Finite range from 0 to 2 hrs
trans = asℝ₊*u"m" # Positive reals, with a unit attached

Two things this PR doesn't currently try to do:

  • Implement any sort of derivatives for these unitful transforms
  • Forward transform from dimensional space to nondimensional (although this can be achieved with an inverse)
  • Provide a mapping from negative real numbers to negative unitful numbers

I'm open to discussion on any of these details--just thought that I would throw together a working draft and see if there would be interest in allowing these kinds of transformations.

@tpapp
Copy link
Owner

tpapp commented Feb 17, 2025

Thanks for providing a draft. I have an alterantive proposal: instead of broadening the API further in this direction, I would handle this with

  1. an elementary transform that just strips quantities from units, or adds a unit to a scalar,
  2. allow chaining transforms together, briefly discussed in the comments of Feature Idea: scaling factor for ShiftedExp  #95.

Implementing is on my near-term TODO list. Then we would add something that exposes as as(u"s") that converts 1 to 1s.

(Further alternative proposals are welcome).

@Ickaser
Copy link
Author

Ickaser commented Feb 17, 2025

I wrote this draft before I saw #95 , and I quite like the idea of adding/stripping units being just one step in a chain--that would mitigate the need to handle the exponential and logistic transformations separately, like I did here

Apropos of that conversation, actually, it probably makes sense to require providing e.g. 1u"s" rather than just u"s", since in general an SI scale (or user-provided units) does not produce a problem that is well-scaled for optimization routines. For example, an optimization problem I use has fit parameters of the order 20u"W/m^2/K" and 3e8u"Ω/m^2"`.

It might make sense to always require a user-defined scale alongside a unit transform, to handle the issue of inappropriate units. It can certainly reuse the scaling transform machinery though.

@tpapp
Copy link
Owner

tpapp commented Feb 18, 2025

I will tackle this chaining extension once we merge #127.

@Ickaser
Copy link
Author

Ickaser commented Mar 5, 2025

If there is a way for me to help with such a refactor, let me know--this will be useful for some of my ongoing work, so I can allocate some time.

@tpapp
Copy link
Owner

tpapp commented Mar 5, 2025

@Ickaser: if you have the time for that, it would be great. I am swamped with other work but I promise to review and engage in discussion if you open a PR.

Broadly, there are two things I was planning on doing:

  1. Introduce a operator for transformations, univariate is fine for now. This is an obvious extension.

  2. Think about a nice API for the building blocks of this. Eg how would the user specify

    a. a simple transformation that adds a unit to a real number (for your use case),
    b. multiplies a real number by a constant,
    c. adds a constant to a real number,

etc. Domain checking (ie compatibility of chained transformations) I would not consider initially at the time of construction, just let it error if it does not work.

It would be great if there was a simple surface for lowering into Base.Fix1 and Base.Fix2, then we could piggyback on that for (2). I haven't explored the options yet.

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

Successfully merging this pull request may close these issues.

2 participants