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

Refactor units::exponent #166

Closed
mpusz opened this issue Sep 15, 2020 · 10 comments
Closed

Refactor units::exponent #166

mpusz opened this issue Sep 15, 2020 · 10 comments
Labels
enhancement New feature or request

Comments

@mpusz
Copy link
Owner

mpusz commented Sep 15, 2020

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

@mpusz mpusz added the enhancement New feature or request label Sep 15, 2020
@mpusz mpusz added this to the v0.7.0 milestone Sep 15, 2020
@mpusz
Copy link
Owner Author

mpusz commented Sep 15, 2020

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

@mpusz
Copy link
Owner Author

mpusz commented Sep 15, 2020

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>;

@kwikius
Copy link
Contributor

kwikius commented Sep 15, 2020

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.

@mpusz
Copy link
Owner Author

mpusz commented Sep 15, 2020

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).

@JohelEGP
Copy link
Collaborator

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.

@mpusz
Copy link
Owner Author

mpusz commented Sep 23, 2020

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
Copy link
Owner Author

mpusz commented Sep 23, 2020

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

@kwikius
Copy link
Contributor

kwikius commented Sep 24, 2020

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

@kwikius
Copy link
Contributor

kwikius commented Oct 6, 2020

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!

@mpusz mpusz removed this from the v0.7.0 milestone May 10, 2021
@mpusz mpusz added this to the v0.9.0 (V2 framework) milestone Dec 5, 2022
@mpusz mpusz mentioned this issue Dec 17, 2022
18 tasks
@mpusz
Copy link
Owner Author

mpusz commented Jun 15, 2023

No longer in V2.

@mpusz mpusz closed this as completed Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants