mp-units icon indicating copy to clipboard operation
mp-units copied to clipboard

Refactor `units::exponent`

Open mpusz opened this issue 5 years ago • 9 comments

Derived dimension types became really long with renaming units::exp to units::exponent. Let's try to find a better alternative.

mpusz avatar Sep 15 '20 07:09 mpusz

A simple case might be just to find a better name.

mpusz avatar Sep 15 '20 07:09 mpusz

Another approach is to change the dimension specification to contain exponent right away. It yields shorter types in error messages.

BEFORE:

../../src/include/units/bits/external/type_traits.h:78:9:   required for the satisfaction of ‘is_derived_from_specialization_of<Dim, DimTemplate>’ [with DimTemplate = units::physical::dim_speed; Dim = units::unknown_dimension<units::exponent<units::physical::si::dim_length, 1, 1>, units::exponent<units::physical::si::dim_time, 1, 1> >]
../../src/include/units/physical/dimensions.h:50:9:   required for the satisfaction of ‘DimensionOf<typename Q::dimension, DimTemplate>’ [with DimTemplate = units::physical::dim_speed; Q = units::quantity<units::unknown_dimension<units::exponent<units::physical::si::dim_length, 1, 1>, units::exponent<units::physical::si::dim_time, 1, 1> >, units::scaled_unit<::_ZTAXtlN5units5ratioELl36ELl1ELl5EEE, units::unknown_coherent_unit>, long int>]
../../src/include/units/physical/dimensions.h:54:9:   required for the satisfaction of ‘QuantityOf<T, units::physical::dim_speed>’ [with T = units::quantity<units::unknown_dimension<units::exponent<units::physical::si::dim_length, 1, 1>, units::exponent<units::physical::si::dim_time, 1, 1> >, units::scaled_unit<::_ZTAXtlN5units5ratioELl36ELl1ELl5EEE, units::unknown_coherent_unit>, long int>]
../../src/include/units/physical/dimensions.h:244:9:   required for the satisfaction of ‘Speed<units::quantity<units::unknown_dimension<units::exponent<units::physical::si::dim_length, 1, 1>, units::exponent<units::physical::si::dim_time, 1, 1> >, units::scaled_unit<units::ratio{36, 1, 5}, units::unknown_coherent_unit>, long int> >’
../../src/include/units/bits/external/type_traits.h:78:45:   in requirements with ‘const volatile units::unknown_dimension<units::exponent<units::physical::si::dim_length, 1, 1>, units::exponent<units::physical::si::dim_time, 1, 1> >* t’
../../src/include/units/bits/external/type_traits.h:78:116: note: the required expression ‘to_base_specialization_of<Type>(t)’ is invalid
   78 | concept is_derived_from_specialization_of = requires(const volatile T* t) { detail::to_base_specialization_of<Type>(t); };
      |                                                                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~

AFTER:

../../src/include/units/bits/external/type_traits.h:78:9:   required for the satisfaction of ‘is_derived_from_specialization_of<Dim, DimTemplate>’ [with DimTemplate = units::physical::dim_speed; Dim = units::unknown_dimension<units::physical::si::dim_length<1, 1>, units::physical::si::dim_time<1, 1> >]
../../src/include/units/physical/dimensions.h:50:9:   required for the satisfaction of ‘DimensionOf<typename Q::dimension, DimTemplate>’ [with DimTemplate = units::physical::dim_speed; Q = units::quantity<units::unknown_dimension<units::physical::si::dim_length<1, 1>, units::physical::si::dim_time<1, 1> >, units::scaled_unit<::_ZTAXtlN5units5ratioELl36ELl1ELl5EEE, units::unknown_coherent_unit>, long int>]
../../src/include/units/physical/dimensions.h:54:9:   required for the satisfaction of ‘QuantityOf<T, units::physical::dim_speed>’ [with T = units::quantity<units::unknown_dimension<units::physical::si::dim_length<1, 1>, units::physical::si::dim_time<1, 1> >, units::scaled_unit<::_ZTAXtlN5units5ratioELl36ELl1ELl5EEE, units::unknown_coherent_unit>, long int>]
../../src/include/units/physical/dimensions.h:244:9:   required for the satisfaction of ‘Speed<units::quantity<units::unknown_dimension<units::physical::si::dim_length<1, 1>, units::physical::si::dim_time<1, 1> >, units::scaled_unit<units::ratio{36, 1, 5}, units::unknown_coherent_unit>, long int> >’
../../src/include/units/bits/external/type_traits.h:78:45:   in requirements with ‘const volatile units::unknown_dimension<units::physical::si::dim_length<1, 1>, units::physical::si::dim_time<1, 1> >* t’
../../src/include/units/bits/external/type_traits.h:78:116: note: the required expression ‘to_base_specialization_of<Type>(t)’ is invalid
   78 | concept is_derived_from_specialization_of = requires(const volatile T* t) { detail::to_base_specialization_of<Type>(t); };
      |                                                                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~

However, to make it happen we will need to change every base dimension to:

template<Unit U, std::intmax_t Num, std::intmax_t Den = 1>
struct dim_length : base_dimension<"L", U, Num, Den> {};
namespace si {

template<std::intmax_t Num, std::intmax_t Den = 1>
struct dim_length : physical::dim_length<metre, Num, Den> {};

}

Every derived dimension to:

template<typename Child, Unit U, DimensionOf<dim_length> L, DimensionOf<dim_time> T, std::intmax_t Num, std::intmax_t Den = 1>
struct dim_speed : derived_dimension<Child, U, Num, Den, L<1>, T<-1>> {};
namespace si {

template<std::intmax_t Num, std::intmax_t Den = 1>
struct dim_speed : physical::dim_speed<dim_speed, metre_per_second, Num, Den, dim_length, dim_time> {};

}

Somehow make the quantity definition and quantity_cast to have a nice user interface. Probably they should get template templates and just put 1 under the hood.

template<Unit U, ScalableNumber Rep = double>
using length = quantity<dim_length, U, Rep>;

Or alternatively Num could be also defined to 1 in the definitions above so:

template<Unit U, ScalableNumber Rep = double>
using length = quantity<dim_length<>, U, Rep>;

rather than:

template<Unit U, ScalableNumber Rep = double>
using length = quantity<dim_length<1>, U, Rep>;

mpusz avatar Sep 15 '20 07:09 mpusz

As said before, I would recommend thinking about decoupling dimension from dependence on unit as it has currently in mpusz/units, then dimension can be used for different measurement systems

In PQS I defined each base_quantity (called in mpusz/units base_dimension I think) as a unique type that conform to base_quantity concept. I provide a base_quantity_exponent concept which allows to raise base_quantity to rational exponent and get at its components, e.g length example here: https://github.com/kwikius/pqs/blob/master/src/include/pqs/types/base_quantity/length.hpp (The complexity there is an attempt get the shortest possible name in error messages , but it could be much simplified. I am not too bothered about the complexity since there are only a few base quantities to be created so it probably wont be an everyday thing)

As part of the interface of a base_quantity I instantiated a global constant of each base_quantity_exponent raised to exponent 1 for reasons explained next https://github.com/kwikius/pqs/blob/master/src/include/pqs/types/base_quantity/length.hpp#L63 (I also provide a typedef for each base_quantity_exponent raised to power 1) https://github.com/kwikius/pqs/blob/master/src/include/pqs/types/base_quantity/length.hpp#L62

I provide operators *,/ etc to work on models of dimension so they can be multiplied , divide, raised to power etc, which makes for very convenient user interface for constructing derived dimensions, e.g dimension abstract_force here: https://github.com/kwikius/pqs/blob/master/src/include/pqs/types/derived_quantity/force.hpp#L9 As each derived dimension is created using the ops, I provide typedefs and global constants for each dimension ( with same abstract_ prefix) as part of the interface to faciliate composition of further derived dimensions by using the runtime operators

There are some more subtleties as to whether to make a dimension a derived class or use a typedef (relating to units output) which I wont go into here.

Anyway it makes for a very satisfying user interface for dimension

EDIT: It may als be possible to convert the interface of many entities to use NTTP dimensions rather than types so that the decltype( expr) syntax would not be needed much or at all.

kwikius avatar Sep 15 '20 11:09 kwikius

Thanks, I will look into it soon (right now I have to prepare for a 2-day CppCon workshop that starts on Monday, and then I have to catch up with my paid work as I spent the last 2-weeks exclusively on units).

mpusz avatar Sep 15 '20 18:09 mpusz

EDIT: It may als be possible to convert the interface of many entities to use NTTP dimensions rather than types so that the decltype( expr) syntax would not be needed much or at all.

This is definitely worth looking into.

JohelEGP avatar Sep 21 '20 23:09 JohelEGP

I would recommend thinking about decoupling dimension from dependence on unit as it has currently in mpusz/units, then dimension can be used for different measurement systems

We can reuse dimension definitions between systems even now. We just share a class template rather than a specific type.

Regarding the decoupling, are you sure that it will be possible to still support all the current use cases (multiple systems support with various units and printing them properly in the text output)?

NTTP usage for dimensions will most probably make error logs much worse as there is no way to make downcasting work with values.

mpusz avatar Sep 23 '20 15:09 mpusz

However, I plan to make some major changes to the design soon that may affect this subject too. Mostly they are to support #48.

mpusz avatar Sep 23 '20 15:09 mpusz

Hi, currently off grid again, hope to get to this next week.

kwikius avatar Sep 24 '20 20:09 kwikius

I would recommend thinking about decoupling dimension from dependence on unit as it has currently in mpusz/units, then dimension can be used for different measurement systems

We can reuse dimension definitions between systems even now. We just share a class template rather than a specific type.

Regarding the decoupling, are you sure that it will be possible to still support all the current use cases (multiple systems support with various units and printing them properly in the text output)?

Decomposing something into simpler components should in general allow more versatile ways to combine them, so I would say yes :)

NTTP usage for dimensions will most probably make error logs much worse as there is no way to make downcasting work with values.

Add this to the list of problems then, caused by the downcasting mechanism!

kwikius avatar Oct 06 '20 10:10 kwikius

No longer in V2.

mpusz avatar Jun 15 '23 06:06 mpusz