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

Fix #580 by using a fixed-point implementation for unit conversions using integer representations #615

Draft
wants to merge 39 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
cf9c05f
wip
burnpanck Sep 15, 2024
3635260
disabled two tests which now trigger #614
burnpanck Sep 15, 2024
74f30da
again; fix C++20 compatibility
burnpanck Sep 15, 2024
2f54c76
addessed most review concerns, fixed CI failure
burnpanck Sep 16, 2024
e003a58
fixed and expanded double_width_int implemenation, tried to fix a bug…
burnpanck Sep 16, 2024
60c94da
one more try
burnpanck Sep 16, 2024
d00b330
fixed pedantic error
burnpanck Sep 16, 2024
99d4315
Merge remote-tracking branch 'upstream/master' into feature/fixed-poi…
burnpanck Nov 5, 2024
653d3d2
fix formatting issues
burnpanck Nov 5, 2024
ed2574f
allow use of __(u)int128, and always use std::bit_width and friends
burnpanck Nov 5, 2024
e688ffc
silence pedantic warning about __int128
burnpanck Nov 5, 2024
55d8fd6
cross-platform silencing of pedantic warning
burnpanck Nov 5, 2024
38dcf64
Merge remote-tracking branch 'upstream/master' into feature/fixed-poi…
burnpanck Nov 6, 2024
ad76149
Apply suggestions from code review
burnpanck Nov 6, 2024
95cc9f3
more review-requested changes, good test-coverage of double_width_int…
burnpanck Nov 6, 2024
5f8eb5c
made hi_ and lo_ private members of double_width_int
burnpanck Nov 6, 2024
1b57404
attempt to fix tests on apple clang
burnpanck Nov 6, 2024
f673619
try to work around issues around friend instantiations of double_widt…
burnpanck Nov 6, 2024
f642d37
fix: gcc-12 friend compilation issue workaround
mpusz Nov 9, 2024
b6a6752
implement dedicated facilities to customise scaling of numbers with m…
burnpanck Nov 10, 2024
647ce6b
fixed a few more details
burnpanck Nov 10, 2024
464ecd4
Merge remote-tracking branch 'upstream/master' into feature/fixed-poi…
burnpanck Nov 10, 2024
e933be7
fix a few issues uncovered in CI
burnpanck Nov 11, 2024
6873c8b
fix formatting
burnpanck Nov 11, 2024
65a0ee4
fix module exports - does not yet inlude other review input
burnpanck Nov 11, 2024
0c1971e
addressed most review input
burnpanck Nov 11, 2024
4ef0210
fix includes (and use curly braces for constructor calls in measurmen…
burnpanck Nov 12, 2024
35ed472
first attempt at generating sparse CI run matrix in python; also, can…
burnpanck Nov 12, 2024
329b9f5
Merge branch 'master' into feature/faster-CI
burnpanck Nov 12, 2024
7fa15d2
fix formatting
burnpanck Nov 12, 2024
e464677
don't test Clang 19 just yet; fix cancel-in-progres
burnpanck Nov 12, 2024
cc9ea9d
add cancel-in-progress to all workflows
burnpanck Nov 12, 2024
a51462c
missing checkout in generate-matrix step
burnpanck Nov 12, 2024
f4c8e90
fix boolean conan options in dynamic CI matrix
burnpanck Nov 12, 2024
01f44c6
heed github warning, and use output file instead of set-output comman…
burnpanck Nov 12, 2024
5713243
fix clang 16
burnpanck Nov 12, 2024
ff11878
exclude clang18+debug from freestanding again
burnpanck Nov 12, 2024
b35e241
fix clang on macos-14 (arm64)
burnpanck Nov 12, 2024
ef0e7b3
Merge branch 'feature/faster-CI' into feature/fixed-point-multiplicat…
burnpanck Nov 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions example/measurement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,37 @@ class measurement {
[[nodiscard]] friend constexpr measurement operator+(const measurement& lhs, const measurement& rhs)
{
using namespace std;
return measurement(lhs.value() + rhs.value(), hypot(lhs.uncertainty(), rhs.uncertainty()));
return measurement{std::in_place, lhs.value() + rhs.value(), hypot(lhs.uncertainty(), rhs.uncertainty())};
}

[[nodiscard]] friend constexpr measurement operator-(const measurement& lhs, const measurement& rhs)
{
using namespace std;
return measurement(lhs.value() - rhs.value(), hypot(lhs.uncertainty(), rhs.uncertainty()));
return measurement{std::in_place, lhs.value() - rhs.value(), hypot(lhs.uncertainty(), rhs.uncertainty())};
}

template<typename To, mp_units::Magnitude M>
[[nodiscard]] constexpr measurement<To> scale(std::type_identity<measurement<To>>, M scaling_factor) const
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type_identity usage here is quite novel. I understand the rationale, but maybe we can find a more traditional way of doing it? @JohelEGP, do you have any ideas here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea comes from boost::hana::type. Would be great if the standard had its own dedicated "type tag".

{
constexpr std::type_identity<To> to_value_type;
return measurement<To>{
std::in_place,
mp_units::scale(to_value_type, scaling_factor, value()),
mp_units::scale(to_value_type, scaling_factor, value()),
};
}

template<mp_units::Magnitude M>
[[nodiscard]] constexpr auto scale(M scaling_factor) const
{
return measurement{
std::in_place,
mp_units::scale(scaling_factor, value()),
mp_units::scale(scaling_factor, value()),
};
}


[[nodiscard]] friend constexpr measurement operator*(const measurement& lhs, const measurement& rhs)
{
const auto val = lhs.value() * rhs.value();
Expand Down Expand Up @@ -127,15 +149,23 @@ class measurement {
private:
value_type value_{};
value_type uncertainty_{};

// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
constexpr measurement(std::in_place_t, value_type val, value_type err) :
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the right way of using in_place. in_place means that we pass a list of arguments for construction of one underlying type.

This is not the case here. We initialize two members with one argument each.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you would like to have in-place like construction for type then probably something like piecewise_construct is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I just wanted a tag to select the private constructor, indicating "I know what I'm doing - I guarantee that err is positive". It's mostly a performance optimisation though, so, for an example, we could just leave it out completely.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can either remove it or use a new dedicated tag. Maybe something similar to:

inline constexpr struct validated_tag {
} validated;

value_(std::move(val)), uncertainty_(std::move(err))
{
}
};

} // namespace


template<typename T>
constexpr bool mp_units::is_scalar<measurement<T>> = true;
template<typename T>
constexpr bool mp_units::is_vector<measurement<T>> = true;

static_assert(mp_units::RepresentationOf<measurement<int>, mp_units::quantity_character::scalar>);
static_assert(mp_units::RepresentationOf<measurement<double>, mp_units::quantity_character::scalar>);

namespace {
Expand Down
2 changes: 2 additions & 0 deletions src/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ add_mp_units_module(
core mp-units-core
HEADERS include/mp-units/bits/constexpr_math.h
include/mp-units/bits/core_gmf.h
include/mp-units/bits/fixed_point.h
include/mp-units/bits/get_associated_quantity.h
include/mp-units/bits/hacks.h
include/mp-units/bits/math_concepts.h
include/mp-units/bits/module_macros.h
include/mp-units/bits/quantity_spec_hierarchy.h
include/mp-units/bits/ratio.h
include/mp-units/bits/scaling.h
include/mp-units/bits/sudo_cast.h
include/mp-units/bits/text_tools.h
include/mp-units/bits/type_list.h
Expand Down
1 change: 1 addition & 0 deletions src/core/include/mp-units/bits/core_gmf.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#ifndef MP_UNITS_IMPORT_STD
#include <array>
#include <bit>
#include <compare>
#include <concepts>
#include <cstddef>
Expand Down
Loading
Loading