uom
uom copied to clipboard
Add photometric units
This PR adds LuminousFlux
and Illuminance
units. Unfortunately they are dimensionally equal to LuminousIntensity
and Luminance
. I tried to provide a Mul
trait implementation to construct LuminousFlux
from LuminousIntensity * SolidAngle
, but try as I might I couldn't get the compiler to allow me to provide the trait. I saw #189 and related issues, but I don't think specialization is necessarily the solution - the compiler knows that LuminousIntensity * SolidAngle
isn't allowed, so it shouldn't be a duplicate implementation... I dunno. Any advice is greatly appreciated.
Thanks for the PR! I'll review in detail over the next few days.
In regards to the Mul
impl the problem is that the Kind
associated type defaults to uom::Kind
. I'm not sure/don't remember why the default isn't to maintain one of the operand's kinds -- if this even makes sense. I'll do some more digging into this as well but I'm not hopeful of a good short-term solution beyond .into()
.
type Output = ...
doesn't specify a kind which means the default is used.
https://github.com/iliekturtles/uom/blob/51a7cbf8238b0faa98b8f882614747d21cfb2f68/src/system.rs#L469-L484
In regards to the
Mul
impl the problem is that theKind
associated type defaults touom::Kind
. I'm not sure/don't remember why the default isn't to maintain one of the operand's kinds -- if this even makes sense. I'll do some more digging into this as well but I'm not hopeful of a good short-term solution beyond.into()
.
type Output = ...
doesn't specify a kind which means the default is used.
Naively, I would expect that by default the operators should only be provided for the case of A<$kind> + B<$kind> = Output<$kind>
, so the example of LuminousIntensity * SolidAngle
should be treated as Kind * SolidAngleKind
and not have a Mul
impl at all unless it's provided by a separate impl. By the way, the compiler error I get is:
error[E0308]: mismatched types
--> src/si/luminous_flux.rs:109:26
|
109 | / &(LuminousIntensity::<V>::new::<li::candela>((*l).clone())
110 | | * SolidAngle::<V>::new::<sa::steradian>((*r).clone())))
| |_________________________________________________________________________________^ expected trait `LuminousFluxKind`, found trait `Kind`
so the compiler knows it's looking for a LuminousFluxKind
. I was hoping that would make it select the alternate Mul
implementation that can produce a LuminousFluxKind
.
Only implementing Mul
and Div
for Kind = uom::Kind
would require way too much work to implement a custom kind (10 Mul
and Div
implementations multiplied by all of the by-val by-ref combinations!) A simple implementation with generics Kl
and Kr
(LHS and RHS's kind) would require specialization.
What might be a possible path to pursue would be to implement Mul
and Div
for all Kind * Kind
combinations. This would require the number of kinds squared impls (72 right now). With a macro this probably wouldn't be too painful:
impl_kind_ops!(Kind * Kind = Kind);
impl_kind_ops!(LuminousIntensityKind * Kind = Kind);
impl_kind_ops!(LuminousIntensityKind * SolidAngleKind = LuminousFluxKind);
// ...
What might be an issue is the Mul
/Div
impls for Quantity
. I tried to do something similar for Dimension
and I can't remember if things didn't work or if there was just an even larger explosion of type bounds that currently exists. The current mul implementation would be changed to use $MulDivAlias
(a shortcut for <A as Mul<V>>::Output
to determine what the output Kind
should be instead of defaulting to uom::Kind
.
impl<Dl, Kl, Dr, Kr, U, V> Mul<Quantity<Dr, U, V>> for Quantity<Dl, U, V>
where
Dl: Dimension<Kind = Kl>,
Dr: Dimension<Kind = Kr>,
// ...
{
type Output = Quantity<
$quantities<$($crate::typenum::$AddSubAlias<Dl::$symbol, Dr::$symbol>,)+ Kind = $MulDivAlias<Kl, Kr>>,
U, V>;
fn mul(/*...*/) -> Self::Output {
// ...
}
}
Because of the scope of this work I'd like to see this effort separated from this PR. We can get Illuminance
and LuminousFlux
merged using the current functionality while finding out a better way to work with Kind
s.
Possibly use #315 as the tracking issue and re-state the original problem.