Support non-arithmetic Rep #52
Labels
⬇️ affects: code (implementation)
Affects implementation details of the code
📁 kind: enhancement
New feature or request
💪 effort: medium
Comments
|
To be clear: we already have experimental support for non- |
chiphogg
added
⬇️ affects: code (implementation)
2 tasks
SagaciousB
pushed a commit
to SagaciousB/au
that referenced
this issue
Nov 22, 2023
Constants (aurora-opensource#90) and symbols (aurora-opensource#43) have a lot of similarities. For example: - Multiplying by a raw number produces a quantity. - Multiplying by a quantity changes the units of the quantity. - They can compose with other instances of the same family (e.g., the product of two constants is a constant). Let's use the term "wrapper" to refer to constants and symbols here. To make each wrapper as easy as possible to implement, I've created some "mixin" classes, each of which adds the full set of multiplication and division operations for a single combination of the wrapper and some other family of types (raw numbers, quantities, other wrappers, etc.). These wrappers use a "CRTP-ish" syntax. But instead of the _type itself_ being the first template parameter (e.g., `Wrapper<Unit>`), we provide the _wrapper_ and _unit_ as two separate parameters (i.e., `Wrapper` and `Unit`). The reason is that we need to know the `Unit`, and providing it explicitly makes it easy to get. These first two parameters are the same for all mixins, which makes the list-of-mixins at the class definition easier to read. The unit tests are based on an example, `UnitWrapper`, which aggregates a variety of properties. The forthcoming "true" classes, `Constant` and `SymbolFor`, will be defined very similarly to this. Finally, a word about the plan for "numeric" inputs. For now, we are restricting to `std::is_arithmetic`. This doesn't mean we don't support other reps; it just means we won't be able to create quantities from them via constants or symbols for a while. The evolution plan is to create a well-defined concept that defines what is a valid rep, and then replace `std::arithmetic` with that concept (see also aurora-opensource#52). Test plan: - [x] Add extensive new unit tests - [x] Manually uncomment each individual "uncomment to test" case
chiphogg
added a commit
that referenced
this issue
Mar 30, 2024
The goal of these `std::is_arithmetic` SFINAE uses was to avoid ambiguous overloads when we multiply a `Quantity` with another `Quantity`. It was only ever a shortcut. It seems to work well in most cases, but recently, a user pointed out that it doesn't help `std::complex`. (To be clear, we could _form_ a `Quantity` with `std::complex` rep, but we couldn't multiply or divide a `Quantity` with a `std::complex` scalar.) This PR takes a different approach. First, we define what constitutes a valid "rep". We're very permissive here, using a "deny-list" approach that filters out empty types, known Au types, and other libraries' units types (which we detect via our `CorrespondingQuantity` trait). Incidentally, this should give us a nice head start on #52. Next, we permit an operation (multiplication or division) depending on how the "candidate scalar" type would interact with the Quantity's rep: simply put, the result must itself both exist, and be a valid rep. The net result should be that we automatically consider a much wider variety of types --- including complex number types from outside the standard library --- to be valid scalars. Crucially, this should also be able to avoid breaking existing code: we are introducing potentially a _lot_ of new overloads for these operators, and we can't allow any to create an ambiguity with existing overloads.
chiphogg
added a commit
that referenced
this issue
Apr 25, 2024
The goal of these `std::is_arithmetic` SFINAE uses was to avoid ambiguous overloads when we multiply a `Quantity` with another `Quantity`. It was only ever a shortcut. It seems to work well in most cases, but recently, a user pointed out that it doesn't help `std::complex`. (To be clear, we could _form_ a `Quantity` with `std::complex` rep, but we couldn't multiply or divide a `Quantity` with a `std::complex` scalar.) This PR takes a different approach. First, we define what constitutes a valid "rep". We're very permissive here, using a "deny-list" approach that filters out empty types, known Au types, and other libraries' units types (which we detect via our `CorrespondingQuantity` trait). Incidentally, this should give us a nice head start on #52. Next, we permit an operation (multiplication or division) depending on how the "candidate scalar" type would interact with the Quantity's rep: simply put, the result must itself both exist, and be a valid rep. The net result should be that we automatically consider a much wider variety of types --- including complex number types from outside the standard library --- to be valid scalars. Crucially, this should also be able to avoid breaking existing code: we are introducing potentially a _lot_ of new overloads for these operators, and we can't allow any to create an ambiguity with existing overloads. Helps #228, but we need more Magnitude support.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As mentioned in #38, it's not our intention to restrict Rep to built-in arithmetic types forever. This issue tracks the idea of defining a separate trait which specifies exactly which properties are needed for a valid Rep.
Note that as part of this effort, we would need to check the impact on compile times.
The text was updated successfully, but these errors were encountered: