`Symbolics.jl` support? about measurements.jl

[giordano]

As I said, I'm not very happy about point 1. Subtypying AbstractFloat instead of Real has several advantages, you get many more methods for free with that instead of <: Real, and point 2 would require having this package depend on Symbolics (assuming that the constraint you want to put would be placed in an inner constructor of the Measurement type), which is also not something I'm very happy about.

[giordano]

Making Measurement <: AbstractFloat instead of <: Real was a decision taken very early on in #1 (as you can tell from the very low number). I'm not going to reverse that decision.

[LebedevRI]

Thank you for replying!

Yes, i have seen that issue when looking at git blame for some of the affected lines of code.

I understand that during the computation, the result inevitably ends up being non-integral,
so excluding Integer types and promoting to AbstractFloat upfront makes sense.
I guess, if someone really wants to avoid FP precision issues, they'd use BigFloat's?

However, it is not so clear-cut given symbolic inputs, because there the result will still be symbolic.

Note that i'm not arguing for allowing everything that is a Real!
Perhaps, could something like this be done instead?

1. Make Measurement{T <: Real} <: Real
2. Prohibit T that is neither AbstractFloat nor Symbolics::Num (i.e. as compared to now, allow Symbolics::Num)
3. At contruction time, promote Real's to AbstractFloat's if they are not Symbolics::Num.

[LebedevRI]

... does that make sense?

[LebedevRI]

Thank you for taking a look!

As I said, I'm not very happy about point 1. Subtypying AbstractFloat instead of Real has several advantages, you get many more methods for free with that instead of <: Real

To be honest, i'm not as familiar with julia, so i'm not fully //sure// doing so is necessary,
i'm only guessing that it is. If it works with just Measurement{T <: Real} <: AbstractFloat,
with Measurement{Num}, then great, no need for Measurement{T <: Real} <: Real...

and point 2 would require having this package depend on Symbolics (assuming that the constraint you want to put would be placed in an inner constructor of the Measurement type), which is also not something I'm very happy about.

Yes indeed, i have thought about that issue, and i fully agree with that pain point,
although i was waiting for you to also point it out before filing JuliaSymbolics/Symbolics.jl#898

[LebedevRI]

Rough proof of concept in #144, with all tests passing, but insufficient test coverage.

As I said, I'm not very happy about point 1. Subtypying AbstractFloat instead of Real has several advantages, you get many more methods for free with that instead of <: Real

To be honest, i'm not as familiar with julia, so i'm not fully //sure// doing so is necessary, i'm only guessing that it is. If it works with just Measurement{T <: Real} <: AbstractFloat, with Measurement{Num}, then great, no need for Measurement{T <: Real} <: Real...

So far it appears that Measurement{T<:Real} <: AbstractFloat is sufficient.

and point 2 would require having this package depend on Symbolics (assuming that the constraint you want to put would be placed in an inner constructor of the Measurement type), which is also not something I'm very happy about.

Yes indeed, i have thought about that issue, and i fully agree with that pain point, although i was waiting for you to also point it out before filing JuliaSymbolics/Symbolics.jl#898

It turns out, we can't depend on Symbolics in the first place, because that creates a dependency cycle.
SciML/RecursiveArrayTools.jl#265 should deal with it.

[LebedevRI]

@giordano ok, i have a question.
From the library's perspective, which one of two possible compromises would be preferred:
1. Separating the functions to support symbolic-typed measurements into separate source files, and only loading them when the Symbolics package is also present, via @require in function __init__().
2. or, depending on //some//, hopefully tiny, package, and being able to have all the functions altogether.

I would guess 2. would result in best code, but will require cooperation from either SciML/RecursiveArrayTools.jl#264 or (preferrably )JuliaSymbolics/Symbolics.jl#898.

Edit: found a way to mix both of these in a nice way.