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

Make AbstractQuantity and AbstractDimensions #24

Merged
merged 31 commits into from
Jun 28, 2023
Merged

Conversation

MilesCranmer
Copy link
Member

@MilesCranmer MilesCranmer commented Jun 12, 2023

This PR adds abstract super types for quantities and dimensions. Almost every function is now defined for the abstract version, even with the constructors set up automagically

julia> struct MyDimensions{R} <: AbstractDimensions{R}
           length::R
           mass::R
           time::R
           scale::R
       end

julia> x = Quantity(1.2, MyDimensions, time=5//2, scale=3)
1.2 s⁵ᐟ² scale³

julia> x^2
1.44 s⁵ scale⁶

julia> typeof(x)
Quantity{Float64, MyDimensions{DynamicQuantities.FixedRational{Int32, 25200}}}

and with an AbstractQuantity (which we can pass our new dimensions type too)

julia> struct MyQuantity{T,D<:AbstractDimensions} <: AbstractQuantity{T,D}
           value::T
           dimensions::D
       end

julia> x = MyQuantity(1.2, MyDimensions, time=5//2, scale=3)
1.2 s⁵ᐟ² scale³

julia> x^2
1.44 s⁵ scale⁶

julia> typeof(x)
MyQuantity{Float64, MyDimensions{DynamicQuantities.FixedRational{Int32, 25200}}}

The individual dimensions are by default the FixedRational type, but you can override this with, e.g

julia> x = MyQuantity(1.2, MyDimensions{Rational{Int}}, time=5//2, scale=3)
1.2 s⁵ᐟ² scale³

or by conversion:

julia> x = convert(MyQuantity{Float64,MyDimensions{Rational{Int}}}, MyQuantity(1.2, MyDimensions, time=5//2, scale=3))
1.2 s⁵ᐟ² scale³

julia> typeof(x)
MyQuantity{Float64, MyDimensions{Rational{Int64}}}

Old version:

This is a draft PR to look at adding some abstract super types for quantities and dimensions.

Nearly all functions are defined for ::AbstractQuantity and ::AbstractDimensions now, so you can overload anything to a particular type.

However, the current interface is a bit awkward it relies on the user defining a quantity function to try to create a Quantity of the correct type:

new_quantity(::Type{<:MyQuantity}, val, dim) = MyQuantity(val, dim)
new_quantity(::Type{<:MyDimensions}, val, dim) = MyQuantity(val, dim)
new_dimensions(::Type{<:MyDimensions}, args...) = MyDimensions(args...)

Maybe this is fine though.

This is because otherwise it would not know what Quantity type to create if you construct a quantity using, e.g., 0.5 * Dimensions(length=1). So you have to create a new AbstractDimensions every time you create an AbstractQuantity.

The other issue is it relies on the fixed DIMENSION_NAMES constant for defining some macros. If a user wants to define a custom dimensions struct with different fields, this would not work. So there should be some way to check this.

Fixed: now you need to override dimension_name(::AbstractDimensions, k::Symbol), and that's it.

So in total, you only need three functions for a custom type to work:

new_quantity
new_dimensions
dimension_name

as well as optionally convert, one, zero, and oneunit. The rest should automatically carry over.

@MilesCranmer MilesCranmer marked this pull request as ready for review June 12, 2023 12:07
@mcabbott
Copy link

What subtypes are envisaged?

I keep wondering whether Quantity(1.23, length=1) should be <: Real. And if I understand right, it may be desirable to make Quantity([1,2,3], length=1) one object, so as not to store Dimensions for every element... perhaps this should be <: AbstractVector. Neither of these could fit under AbstractQuantity; they would instead need methods to be defined for some AnyQuantity = Union{RealQuantity, QuantityArray}.

@MilesCranmer
Copy link
Member Author

I could see someone wanting to use fewer fields in Dimensions. For example, someone only needing length, mass, and time could write:

using DynamicQuantities
import DynamicQuantities: new_dimensions, new_quantity, dimension_name

struct MyDimensions{R} <: AbstractDimensions{R}
    length::R
    mass::R
    time::R

    MyDimensions(args::_R...) where {_R} = new{_R}(args...)
end

struct MyQuantity{T,R} <: AbstractQuantity{T,R}
    value::T
    dimensions::MyDimensions{R}

    MyQuantity(x, d::MyDimensions{_R}) where {_R}  = new{typeof(x), _R}(x, d)
end

new_dimensions(::Type{<:MyDimensions}, l...) = MyDimensions(l...)
new_quantity(::Type{<:MyQuantity}, v, d) = MyQuantity(v, d)
new_quantity(::Type{<:MyDimensions}, v, d) = MyQuantity(v, d)
dimension_name(::MyDimensions, k::Symbol) = (length="m", mass="kg", time="s")[k]

which seems to work:

julia> q1 = MyQuantity(0.5, MyDimensions(0, 3, 2))
0.5 kg³ s²

julia> q2 = MyQuantity(0.3, MyDimensions(-1, 5, 2))
0.3 m⁻¹ kg⁵ s²

julia> q1 * q2
0.15 m⁻¹ kg⁸ s⁴

@MilesCranmer
Copy link
Member Author

Neither of these could fit under AbstractQuantity; they would instead need methods to be defined for some AnyQuantity = Union{RealQuantity, QuantityArray}.

Yeah, this part is tricky...

@mcabbott
Copy link

wanting to use fewer fields in Dimensions

Yes. I missed that this PR did that too, was only thinking about AbstractQuantity above.

The existing Dimensions is SI + rational, plausibly you could decide you can get away with three Int8 powers for some problem (a subset) or might want something more exotic (like c=1 units, or some imperial or financial units).

@MilesCranmer
Copy link
Member Author

The existing Dimensions is SI + rational, plausibly you could decide you can get away with three Int8 powers for some problem (a subset) or might want something more exotic (like c=1 units, or some imperial or financial units).

Just to clarify - the existing Dimensions already permits you to use Int8 rather than a rational (or any other type for the power).

But yeah I agree it could be useful to have other types of Dimensions objects. The question is how to best design the super type to allow both user-defined Dimensions as well as AbstractArray-based Quantities...

src/types.jl Outdated Show resolved Hide resolved
src/types.jl Outdated Show resolved Hide resolved
Base automatically changed from additional-utils to main June 14, 2023 18:20
@j-fu
Copy link
Contributor

j-fu commented Jun 14, 2023

What subtypes are envisaged?

I already posted this in one of the other discussions:
I would like to enable linear system solution with quantities.
This would need a couple of tweaks which are (probably rightfully) rejected for Quantity. By introduceing e.g. XQuantity we could enable dense and sparse linear system solution via:

# needed for dense A\b
Base.zero(::Type{XQuantity{T,R}}) where {T,R} = XQuantity(zero(T))
Base.one(::Type{XQuantity{T,R}}) where {T,R} = XQuantity(one(T))
adjoint(q::XQuantity{T,R}) where {T,R} = XQuantity(adjoint(q.value),q.dimensions)

# needed for sparspaklu(A)\b
Base.zero(::XQuantity{T,R}) where {T,R} = XQuantity(zero(T))
Base.one(::XQuantity{T,R}) where {T,R} = XQuantity(one(T))
Base.isless(q,r) = q.dimensions==r.dimensions && q.value<r.value
function Base.:+(l::XQuantity, r::XQuantity)
    if iszero(l.value)
        r
    elseif iszero(r.value)
        l
    else
        dimension(l) == dimension(r) ? new_quantity(typeof(l), ustrip(l) + ustrip(r), dimension(l)) : throw(DimensionError(l, r))
    end
end

@github-actions
Copy link
Contributor

github-actions bot commented Jun 14, 2023

Benchmark Results

main a365da0... t[main]/t[a365da0...]
creation/Quantity(x) 4 ± 0.3 ns 4.1 ± 0.3 ns 0.976
creation/Quantity(x, length=y) 4.7 ± 0.1 ns 4.5 ± 0.3 ns 1.04
time_to_load 0.144 ± 0.016 s 0.167 ± 0.0055 s 0.864
with_numbers/*real 4.3 ± 0.2 ns 3.3 ± 0.2 ns 1.3
with_numbers/^int 26.1 ± 4.5 ns 25.5 ± 3.3 ns 1.02
with_numbers/^int * real 26.2 ± 3.7 ns 25.9 ± 3.5 ns 1.01
with_quantity/+y 7.4 ± 0.3 ns 6.7 ± 0.3 ns 1.1
with_quantity//y 4.9 ± 0.1 ns 4.1 ± 0.2 ns 1.2
with_self/dimension 1.7 ± 0.2 ns 1.5 ± 0.1 ns 1.13
with_self/inv 4.4 ± 0.4 ns 3.8 ± 0.3 ns 1.16
with_self/ustrip 1.8 ± 0.1 ns 1.7 ± 0.1 ns 1.06

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

@MilesCranmer
Copy link
Member Author

I think this sounds feasible to support. Does the current abstract type interface work for implementing this? Or is there something else that is needed to get it working?

@MilesCranmer
Copy link
Member Author

MilesCranmer commented Jun 14, 2023

I changed the functions a user needs to overload, to make things simpler. Now it's just these ones:

dimension_name(::Dimensions, k::Symbol) = (length="m", mass="kg", time="s", current="A", temperature="K", luminosity="cd", amount="mol")[k]
quantity_constructor(::Type{<:Union{Quantity,Dimensions}}) = Quantity
dimension_constructor(::Type{<:Union{Quantity,Dimensions}}) = Dimensions

With the AbstractQuantity type having .value::T and .dimensions::AbstractDimensions{R}

I'm using static_fieldnames from Tricks.jl so that the various functions get to compile the fields in, which speeds things up.


edit: I suppose we could somehow get this constructor info automatically... by looking at the field types?

@j-fu
Copy link
Contributor

j-fu commented Jun 14, 2023

Does the current abstract type interface work for implementing this? Or is there something else that is needed to get it working?

Yes - I checked this. Not yet sure where I will land that code, though.

@MilesCranmer
Copy link
Member Author

Okay, let me know if there’s something missing in the abstract interface.

Also I think there might be a way to make these required functions completely optional.

@MilesCranmer
Copy link
Member Author

MilesCranmer commented Jun 15, 2023

Sorry for the spam. Now I'm wondering if instead of an abstract type, AbstractQuantity should have been a trait using SimpleTraits.jl. As it stands, we can't easily create a QuantityArray because Julia doesn't have multiple inheritance, and there is no trait-based version of AbstractArray.

With SimpleTraits.jl, this would look like:

@traitfn Base.:+(x::::AbstractQuantity) = ...

where we would have

@traitimpl AbstractQuantity{X} <- quantity_like(X)

where quantity_like(X) checks that type X defines a .value and a .dimensions, and that the .dimensions satisfies the trait AbstractDimensions{X}.

Then we can just create a QuantityArray{A<:AbstractArray,R} <: AbstractArray, and have

@traitimpl AbstractQuantity{QuantityArray}

What do you think @mcabbott?

@SBuercklin
Copy link

SBuercklin commented Jun 15, 2023

From the thread where this was proposed on Discourse, there was discussion of using the Dimensions{R} type to be statically inferable. I just want to point out that this wouldn't be the case, as Val(1) and Val(0) are distinct types to the compiler.

If supporting this statically-inferable idea is part of the intention, it's not clear to me that a single type parameter in AbstractDimensions{R} can capture both the dynamic and static use-cases. If permitting the use of type-inferable dimensions is part of the plan, wouldn't it make more sense to remove the type parameter on AbstractDimensions entirely?

@MilesCranmer
Copy link
Member Author

MilesCranmer commented Jun 15, 2023

From the thread where this was proposed on Discourse, there was discussion of using the Dimensions{R} type to be statically inferable. I just want to point out that this wouldn't be the case, as Val(1) and Val(0) are distinct types to the compiler.

If supporting this statically-inferable idea is part of the intention, it's not clear to me that a single type parameter in AbstractDimensions{R} can capture both the dynamic and static use-cases. If permitting the use of type-inferable dimensions is part of the plan, wouldn't it make more sense to remove the type parameter on AbstractDimensions entirely?

Ah, apologies for the confusion – that comment on discourse was just an idea, but is unrelated to the changes implemented by this PR. Indeed, statically-inferrable dimensions are not possible with the current types (and probably better left to Unitful). This PR is for if you would like to use custom dimension spaces, but with otherwise the same types with variable dimensions.

@odow
Copy link
Contributor

odow commented Jun 15, 2023

cc @trulsf, x-ref trulsf/UnitJuMP.jl#18

@MilesCranmer
Copy link
Member Author

MilesCranmer commented Jun 16, 2023

Do you have a specific branch I could follow along? I am interested in making the abstract interface compatible with your use case.

What would your ideal API be? e.g., in this example:

julia> using DynamicQuantities, JuMP, HiGHS

julia> m = Model(HiGHS.Optimizer);

julia> set_silent(m)

julia> @variable(m, x >= 0, u"m/s")
x [1.0 m s⁻¹]

julia> @variable(m, y >= 0, u"km/h")
y [0.2777777777777778 m s⁻¹]

julia> @variable(m, z, Bin, Dimensions())
z [1.0 ]

julia> max_speed = 4u"km/s"
4000.0 m s⁻¹

julia> @constraint(m, x + y  <= max_speed * z)
x + 0.2777777777777778 y - 4000 z <= 0 [m s⁻¹]

julia> obj = @objective(m, Max, x + 3y)
0.8333333333333334 y + x [m s⁻¹]

julia> optimize!(m)

julia> println("obj = $(value(obj))")
obj = 12000.0 m s⁻¹

julia> println("x = $(value(x))")
x = 0.0 m s⁻¹

julia> println("y = $(value(y))")
y = 4000.0 m s⁻¹

julia> println("z = $(value(z))")
z = 1.0

what would the preferred outputs be?

@MilesCranmer
Copy link
Member Author

Also - I tried defining an IsQuantity{X} trait with SimpleTraits.jl, so that one could inherit methods from other types instead of AbstractQuantity. I realized this is not going to work, because it requires running @traitfn on functions in Base (like (+) to define it for types matching IsQuantity{X}). Due to the way traits in Julia work, this invalidates all generic methods on functions in Base by which is maybe not a great idea.

For example,

@traitfn Base.eltype(::Type{D}) where {D; IsQuantity{D}} = ...

would actually generate a function Base.eltype(::Type{D}) where {D} - thus overwriting the stdlib one.

So we have to stick to abstract types until Julia starts allowing traits or multiple inheritance.

@MilesCranmer
Copy link
Member Author

MilesCranmer commented Jun 24, 2023

If there are no other comments on this abstract type system, I may try to merge things later this week.

@MilesCranmer
Copy link
Member Author

MilesCranmer commented Jun 24, 2023

One quick update: I also just refactored all quantities types to be parametrically typed based on their entire dimension type, rather than the element type of the dimensions. I think this is a much more general option, and does not cost us anything because the space of dimensions is still fixed throughout any calculation.

So, for example,

struct Quantity{T,D<:AbstractDimensions} <: AbstractQuantity{T,D}
    value::T
    dimensions::D
end

whereas before it was the more rigid

struct Quantity{T,R} <: AbstractQuantity{T,R}
    value::T
    dimensions::Dimensions{R}
end

This means that you could use the same Quantity object as implemented here, but with a different AbstractDimensions types.


The downside is that you can no longer automatically create quantities by multiplying a dimension object, like:

0.5 * Dimensions(length=1)

But I think this is in opposition to the abstract interface anyways (how do you safely associate a quantity type from a dimensions type?), so it's better kept out.

@MilesCranmer
Copy link
Member Author

Thanks everyone for the feedback on this PR! Merging now.

@MilesCranmer MilesCranmer merged commit d36e875 into main Jun 28, 2023
@MilesCranmer MilesCranmer deleted the abstract-types-2 branch June 28, 2023 13:52
MilesCranmer added a commit that referenced this pull request Jul 9, 2023
[Diff since v0.4.0](v0.4.0...v0.5.0)

**Closed issues:**
- Use `Int64 / C` as default (#20)

**Merged pull requests:**
- Make `AbstractQuantity` and `AbstractDimensions` (#24) (@MilesCranmer)
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.

5 participants