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

Documentation - QuantityMaker(QuantityD) troubleshooting #288

Open
connorjak opened this issue Aug 23, 2024 · 3 comments
Open

Documentation - QuantityMaker(QuantityD) troubleshooting #288

connorjak opened this issue Aug 23, 2024 · 3 comments
Labels
⬇️ affects: code (interfaces) Affects the way end users will interact with the library 📁 kind: cleanup Making the library nicer in some way, without affecting functionality much or at all 💪 effort: small
Milestone

Comments

@connorjak
Copy link

When work in progress while converting code to use units, I notice a lot of my errors narrow down to this:

QuantityD<Radians> recently_returning_quantityd_function()
{
  ...
}

auto mu = recently_returning_quantityd_function();
auto mu_2 = radians(mu); //quantitymaker on a quantity

This case isn't covered by the troubleshooting documentation here: https://aurora-opensource.github.io/au/main/troubleshooting. The errors in MSVC are also a bit difficult to use to discover the issue. I'm not sure if there's a code change to make for better asserts for this, but maybe this case should be added to the troubleshooting documentation.

@connorjak
Copy link
Author

Here is a portion of the relevant MSVC 14.40 error log:

C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.40.33807\include\type_traits(1338): error C2794: 'type': is not a member of any direct or indirect base class of 'std::common_type<au::Quantity<au::Radians,double>,double>'
  C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.40.33807\include\type_traits(1338): note: the template instantiation context (the oldest one first) is
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\server\CoreMath\Orbital.h(215): note: see reference to function template instantiation 'au::Quantity<au::Radians,double>::Quantity<au::Radians,au::Quantity<au::Radians,double>,void>(au::Quantity<au::Radians,au::Quantity<au::Radians,double>>)' being compiled
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\server\CoreMath\Orbital.h(215): note: see the first reference to 'au::Quantity<au::Radians,double>::Quantity' in 'st_orbit::getElemsfromState'
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3736): note: see reference to function template instantiation 'auto au::Quantity<au::Radians,au::Quantity<au::Radians,double>>::as<double,au::Radians,void>(NewUnit) const' being compiled
          with
          [
              NewUnit=au::Radians
          ]
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3761): note: see reference to alias template instantiation 'std::common_type_t<au::Quantity<au::Radians,double>,double>' being compiled
C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au\au_all_units.h(3761): error C2938: 'std::common_type_t' : Failed to specialize alias template
C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au\au_all_units.h(3765): error C2672: 'au::detail::apply_magnitude': no matching overloaded function found
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3631): note: could be 'T au::detail::apply_magnitude(const T &,au::Magnitude<BP2s...>)'
C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au\au_all_units.h(3764): error C2672: 'au::make_quantity': no matching overloaded function found
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3652): note: could be 'auto au::make_quantity(T)'
C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au\au_all_units.h(3736): error C2665: 'au::Quantity<au::Radians,double>::Quantity': no overloaded function could convert all the argument types
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(4013): note: could be 'au::Quantity<au::Radians,double>::Quantity(double)'
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3736): note: 'au::Quantity<au::Radians,double>::Quantity(double)': cannot convert argument 1 from 'void' to 'double'
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3736): note: Expressions of type void cannot be converted to other types
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3747): note: or       'au::Quantity<au::Radians,double>::Quantity(au::Zero)'
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3736): note: 'au::Quantity<au::Radians,double>::Quantity(au::Zero)': cannot convert argument 1 from 'void' to 'au::Zero'
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3736): note: Expressions of type void cannot be converted to other types
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3755): note: or       'au::Quantity<au::Radians,double>::Quantity(T &&)'
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3736): note: 'au::Quantity<au::Radians,double>::Quantity(T &&)': could not deduce template argument for '__formal'
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3754): note: 'au::CorrespondingQuantityT' : Failed to specialize alias template
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3686): note: 'Unit': is not a member of any direct or indirect base class of 'au::CorrespondingQuantity<T>'
          with
          [
              T=void
          ]
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3744): note: or       'au::Quantity<au::Radians,double>::Quantity(au::Quantity<OtherUnit,OtherRep>)'
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3736): note: 'au::Quantity<au::Radians,double>::Quantity(au::Quantity<OtherUnit,OtherRep>)': could not deduce template argument for 'au::Quantity<OtherUnit,OtherRep>' from 'void'
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3735): note: or       'au::Quantity<au::Radians,double>::Quantity(au::Quantity<OtherUnit,OtherRep>)'
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3736): note: 'au::Quantity<au::Radians,double>::Quantity(au::Quantity<OtherUnit,OtherRep>)': could not deduce template argument for 'au::Quantity<OtherUnit,OtherRep>' from 'void'
  C:\Users\conno\Documents\SC\UE5\SC_Platform\Compute_Server\externals\Au/au_all_units.h(3736): note: while trying to match the argument list '(void)'

@chiphogg
Copy link
Contributor

chiphogg commented Aug 23, 2024

Thanks for calling this out! As you say, there are really two issues here: better error messages for trying to "quantitify" a quantity, and giving our troubleshooting guide a refresh. I think we should do both.

At the moment, I can't pick up new work items until I have my CppCon 2024 slides up to "dry run" standards. The dry run will probably be Sep. 2 or 3. In the meantime, I'll write down the things I think we should do to close this item out.

  • Add static_assert in Quantity and QuantityPoint for IsValidRep
  • Add operator() specialization in QuantityMaker and QuantityPointMaker for Quantity and QuantityPoint, to give very direct error message
  • Add new section to the troubleshooting guide for this kind of error (perhaps doing Provide more flexible alternative to integer_quotient() #253 at the same time)

The troubleshooting guide is definitely due for a refresh. I'll probably do that aspect of it at the same time as #253 (which I've solved in my local client, and just need to get out for review).

So, I'd say this has a pretty good chance of being addressed sometime in September.

@chiphogg chiphogg added 📁 kind: cleanup Making the library nicer in some way, without affecting functionality much or at all 💪 effort: small ⬇️ affects: code (interfaces) Affects the way end users will interact with the library labels Aug 23, 2024
@chiphogg chiphogg added this to the 0.3.6 milestone Aug 23, 2024
chiphogg added a commit that referenced this issue Sep 21, 2024
This covers situations where somebody calls a `QuantityMaker`, or
`QuantityPointMaker`, on a value that isn't a valid rep.  (One of the
most common use cases is when the value is _already_ a core Au type, a
`Quantity` or `QuantityMaker`, and the call is redundant.

The first approach is a blanket approach: we start enforcing the "valid
rep" definition that we had previously added.  This already makes the
error messages a lot nicer and shorter.

We then go further for the most common use case of redundant maker
calls.  By adding templated overloads to each maker for `Quantity` and
`QuantityPoint`, we can directly tell users what they did wrong.  In
order to implement these overloads, we needed an `AlwaysFalse` utliity,
which we added along with unit tests.

Helps #288.  We will follow up to close this out by updating the
troubleshooting guide.
@chiphogg
Copy link
Contributor

@connorjak, check out this comment to see the before-and-after on the error messages for the implementation that's currently under review. (Fair warning: you'll have to scroll through a lot of "before" to even get to the "after"!)

chiphogg added a commit that referenced this issue Sep 23, 2024
This covers situations where somebody calls a `QuantityMaker`, or
`QuantityPointMaker`, on a value that isn't a valid rep.  (One of the
most common use cases is when the value is _already_ a core Au type, a
`Quantity` or `QuantityMaker`, and the call is redundant.

The first approach is a blanket approach: we start enforcing the "valid
rep" definition that we had previously added.  This already makes the
error messages a lot nicer and shorter.

We then go further for the most common use case of redundant maker
calls.  By adding templated overloads to each maker for `Quantity` and
`QuantityPoint`, we can directly tell users what they did wrong.  In
order to implement these overloads, we needed an `AlwaysFalse` utliity,
which we added along with unit tests.

Helps #288.  We will follow up to close this out by updating the
troubleshooting guide.

---------

Co-authored-by: Geoffrey Viola <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⬇️ affects: code (interfaces) Affects the way end users will interact with the library 📁 kind: cleanup Making the library nicer in some way, without affecting functionality much or at all 💪 effort: small
Projects
None yet
Development

No branches or pull requests

2 participants