From 1ebd328a63591dd259e9f5f65e7a56bb227c1bf3 Mon Sep 17 00:00:00 2001 From: "Alexander J. Pfleger" <70842573+AJPfleger@users.noreply.github.com> Date: Sat, 7 Dec 2024 12:48:54 +0100 Subject: [PATCH 1/3] fix: typo in variable in vertex muon scan script (#3959) ## Summary by CodeRabbit - **Bug Fixes** - Corrected error handling in the vertex_mu_scan script to properly append NaN values to the time array. - **Chores** - Made spelling corrections in the CI/codespell_ignore.txt file to enhance clarity and correctness. --- CI/codespell_ignore.txt | 1 - Examples/Scripts/vertex_mu_scan.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/CI/codespell_ignore.txt b/CI/codespell_ignore.txt index 13a3328440f..40dd321d0f5 100644 --- a/CI/codespell_ignore.txt +++ b/CI/codespell_ignore.txt @@ -5,7 +5,6 @@ coner dthe iself sortings -fime gaus te parm diff --git a/Examples/Scripts/vertex_mu_scan.py b/Examples/Scripts/vertex_mu_scan.py index ee8977fe9df..0de15b20097 100755 --- a/Examples/Scripts/vertex_mu_scan.py +++ b/Examples/Scripts/vertex_mu_scan.py @@ -39,7 +39,7 @@ def main(files: List[Path], output: str, title: str = ""): if time_file.exists(): time = numpy.append(time, float(time_file.read_text())) else: - fime.append(float("nan")) + time.append(float("nan")) rf = uproot.open(f"{file}:vertexing") From f16ed67a706a0b1ab3ea84291a8d737f705881e6 Mon Sep 17 00:00:00 2001 From: Stephen Nicholas Swatman Date: Sat, 7 Dec 2024 14:57:20 +0100 Subject: [PATCH 2/3] fix: Checkout branch in ExaTrk CI (#3967) Currently, the ExaTrk CI is trying to run a dependency script before having properly checked out the right branch, possible causing a CI error. This commit ensures that the target branch is properly checked out before trying to use any files in the repository. ## Summary by CodeRabbit - **Chores** - Improved CI/CD pipeline configuration for better build and test job management. - Standardized caching mechanisms and dependency handling across jobs. - Enhanced clarity and maintainability of the configuration. --- .gitlab-ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e278b18eade..25aff340111 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -155,7 +155,10 @@ test_exatrkx_unittests: - apt-get update -y - git clone $CLONE_URL src - - source src/CI/dependencies.sh + - cd src + - git checkout $HEAD_SHA + - source CI/dependencies.sh + - cd .. - ctest --test-dir build -R ExaTrkX test_exatrkx_python: From 9067679f3908b08e8ce55c5c61d75fde0b9061a9 Mon Sep 17 00:00:00 2001 From: Stephen Nicholas Swatman Date: Sat, 7 Dec 2024 16:29:01 +0100 Subject: [PATCH 3/3] feat: Add monadic functions to `Result` (#3957) In order to make the `Result` type easier to use, this commit adds three new functions: * `Result::value_or` allows the user to obtain the value or a provided default value. * `Result::transform` models functorial mapping, allowing users to modify values inside results. * `Result::and_then` models monadic binding, allowing users to build complex chains of actions on results. Implemented are lvalue and rvalue versions of these functions as well as tests. ![image](https://github.com/user-attachments/assets/591e64a2-e4fb-4ce6-9d34-166b28f7a837) ## Summary by CodeRabbit - **New Features** - Enhanced `Result` class with new methods for improved error handling and value transformation: - `value_or`: Retrieve valid value or default. - `transform`: Apply a function to the valid value. - `and_then`: Chain functions based on the result's validity. - **Tests** - Added comprehensive test cases to validate the new functionalities of the `Result` class, ensuring reliability and correctness: - `ValueOrResult`: Validates the `value_or` method. - `TransformResult`: Assesses the `transform` method. - `AndThenResult`: Evaluates the `and_then` method. --- Core/include/Acts/Utilities/Result.hpp | 141 ++++++++++++++++++ .../UnitTests/Core/Utilities/ResultTests.cpp | 88 +++++++++++ 2 files changed, 229 insertions(+) diff --git a/Core/include/Acts/Utilities/Result.hpp b/Core/include/Acts/Utilities/Result.hpp index 9410fa0edfe..475019fc99b 100644 --- a/Core/include/Acts/Utilities/Result.hpp +++ b/Core/include/Acts/Utilities/Result.hpp @@ -30,6 +30,9 @@ class Result { Result(std::variant&& var) : m_var(std::move(var)) {} public: + using ValueType = T; + using ErrorType = E; + /// Default construction is disallowed. Result() = delete; @@ -172,6 +175,144 @@ class Result { return std::move(std::get(m_var)); } + /// Retrieves the valid value from the result object, or returns a default + /// value if no valid value exists. + /// + /// @param[in] v The default value to use if no valid value exists. + /// @note This is the lvalue version. + /// @note This function always returns by value. + /// @return Either the valid value, or the given substitute. + template + std::conditional_t, const T&, T> value_or(U&& v) const& + requires(std::same_as, T>) + { + if (ok()) { + return value(); + } else { + return std::forward(v); + } + } + + /// Retrieves the valid value from the result object, or returns a default + /// value if no valid value exists. + /// + /// @param[in] v The default value to use if no valid value exists. + /// @note This is the rvalue version which moves the value out. + /// @note This function always returns by value. + /// @return Either the valid value, or the given substitute. + template + T value_or(U&& v) && + requires(std::same_as, T>) + { + if (ok()) { + return std::move(*this).value(); + } else { + return std::forward(v); + } + } + + /// Transforms the value contained in this result. + /// + /// Applying a function `f` to a valid value `x` returns `f(x)`, while + /// applying `f` to an invalid value returns another invalid value. + /// + /// @param[in] callable The transformation function to apply. + /// @note This is the lvalue version. + /// @note This functions is `fmap` on the functor in `A` of `Result`. + /// @return The modified valid value if exists, or an error otherwise. + template + auto transform(C&& callable) const& + requires std::invocable + { + using CallableReturnType = decltype(std::declval()(std::declval())); + using R = Result, E>; + if (ok()) { + return R::success(callable(value())); + } else { + return R::failure(error()); + } + } + + /// Transforms the value contained in this result. + /// + /// Applying a function `f` to a valid value `x` returns `f(x)`, while + /// applying `f` to an invalid value returns another invalid value. + /// + /// @param[in] callable The transformation function to apply. + /// @note This is the rvalue version. + /// @note This functions is `fmap` on the functor in `A` of `Result`. + /// @return The modified valid value if exists, or an error otherwise. + template + auto transform(C&& callable) && + requires std::invocable + { + using CallableReturnType = decltype(std::declval()(std::declval())); + using R = Result, E>; + if (ok()) { + return R::success(callable(std::move(*this).value())); + } else { + return R::failure(std::move(*this).error()); + } + } + + /// Bind a function to this result monadically. + /// + /// This function takes a function `f` and, if this result contains a valid + /// value `x`, returns `f(x)`. If the type of `x` is `T`, then `f` is + /// expected to accept type `T` and return `Result`. In this case, + /// `transform` would return the unhelpful type `Result>`, so + /// `and_then` strips away the outer layer to return `Result`. If the + /// value is invalid, this returns an invalid value in `Result`. + /// + /// @param[in] callable The transformation function to apply. + /// @note This is the lvalue version. + /// @note This functions is `>>=` on the functor in `A` of `Result`. + /// @return The modified valid value if exists, or an error otherwise. + template + auto and_then(C&& callable) const& + requires std::invocable + { + using R = decltype(std::declval()(std::declval())); + + static_assert(std::same_as, + "bind must take a callable with the same error type"); + + if (ok()) { + return callable(value()); + } else { + return R::failure(error()); + } + } + + /// Bind a function to this result monadically. + /// + /// This function takes a function `f` and, if this result contains a valid + /// value `x`, returns `f(x)`. If the type of `x` is `T`, then `f` is + /// expected to accept type `T` and return `Result`. In this case, + /// `transform` would return the unhelpful type `Result>`, so + /// `and_then` strips away the outer layer to return `Result`. If the + /// value is invalid, this returns an invalid value in `Result`. + /// + /// @param[in] callable The transformation function to apply. + /// @note This is the rvalue version. + /// @note This functions is `>>=` on the functor in `A` of `Result`. + /// @return The modified valid value if exists, or an error otherwise. + template + auto and_then(C&& callable) && + requires std::invocable + { + using R = decltype(std::declval()(std::declval())); + + static_assert(std::same_as, + "bind must take a callable with the same error type"); + + if (ok()) { + return callable(std::move(*this).value()); + } else { + return R::failure(std::move(*this).error()); + } + } + private: std::variant m_var; diff --git a/Tests/UnitTests/Core/Utilities/ResultTests.cpp b/Tests/UnitTests/Core/Utilities/ResultTests.cpp index 0a873f16d4c..748e0d7211e 100644 --- a/Tests/UnitTests/Core/Utilities/ResultTests.cpp +++ b/Tests/UnitTests/Core/Utilities/ResultTests.cpp @@ -333,6 +333,94 @@ BOOST_AUTO_TEST_CASE(BoolResult) { BOOST_CHECK_EQUAL(res.error(), MyError::Failure); } +BOOST_AUTO_TEST_CASE(ValueOrResult) { + using Result = Result; + + Result res = Result::success(5); + BOOST_CHECK_EQUAL(res.value_or(42), 5); + + res = Result::failure(MyError::Failure); + BOOST_CHECK_EQUAL(res.value_or(42), 42); + + BOOST_CHECK_EQUAL(Result::success(5).value_or(42), 5); + BOOST_CHECK_EQUAL(Result::failure(MyError::Failure).value_or(42), 42); + + int val = 25; + const int cval = 30; + + BOOST_CHECK_EQUAL(Result::success(5).value_or(val), 5); + BOOST_CHECK_EQUAL(Result::success(5).value_or(cval), 5); + BOOST_CHECK_EQUAL(Result::failure(MyError::Failure).value_or(val), 25); + BOOST_CHECK_EQUAL(Result::failure(MyError::Failure).value_or(cval), 30); + + res = Result::success(5); + + BOOST_CHECK_EQUAL(res.value_or(val), 5); + BOOST_CHECK_EQUAL(&(res.value_or(val)), &res.value()); + BOOST_CHECK_EQUAL(res.value_or(cval), 5); + BOOST_CHECK_EQUAL(&(res.value_or(cval)), &res.value()); + + res = Result::failure(MyError::Failure); + + BOOST_CHECK_EQUAL(res.value_or(val), 25); + BOOST_CHECK_EQUAL(res.value_or(cval), 30); + BOOST_CHECK_EQUAL(&(res.value_or(val)), &val); + BOOST_CHECK_EQUAL(&(res.value_or(cval)), &cval); +} + +BOOST_AUTO_TEST_CASE(TransformResult) { + using Result = Result; + + auto f1 = [](int x) { return 2 * x; }; + + Result res = Result::success(5); + Result res2 = res.transform(f1); + BOOST_CHECK(res2.ok()); + BOOST_CHECK_EQUAL(*res2, 10); + + res = Result::failure(MyError::Failure); + res2 = res.transform(f1); + BOOST_CHECK(!res2.ok()); + + BOOST_CHECK(Result::success(5).transform(f1).ok()); + BOOST_CHECK_EQUAL(Result::success(5).transform(f1).value(), 10); + + BOOST_CHECK(!Result::failure(MyError::Failure).transform(f1).ok()); +} + +BOOST_AUTO_TEST_CASE(AndThenResult) { + using Result1 = Result; + using Result2 = Result; + + auto f1 = [](int x) -> Result2 { + return Result2::success("hello " + std::to_string(x)); + }; + auto f2 = [](int) -> Result2 { return Result2::failure(MyError::Failure); }; + + Result1 res = Result1::success(5); + Result2 res2 = res.and_then(f1); + BOOST_CHECK(res2.ok()); + BOOST_CHECK_EQUAL(*res2, "hello 5"); + + res2 = res.and_then(f2); + BOOST_CHECK(!res2.ok()); + + res = Result1::failure(MyError::Failure); + res2 = res.and_then(f1); + BOOST_CHECK(!res2.ok()); + + res2 = res.and_then(f2); + BOOST_CHECK(!res2.ok()); + + BOOST_CHECK(Result1::success(5).and_then(f1).ok()); + BOOST_CHECK_EQUAL(Result1::success(5).and_then(f1).value(), "hello 5"); + + BOOST_CHECK(!Result1::success(5).and_then(f2).ok()); + + BOOST_CHECK(!Result1::failure(MyError::Failure).and_then(f1).ok()); + + BOOST_CHECK(!Result1::failure(MyError::Failure).and_then(f2).ok()); +} BOOST_AUTO_TEST_SUITE_END() } // namespace Acts::Test