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

use std::expected to handle burn failures? #1439

Open
BenWibking opened this issue Jan 3, 2024 · 4 comments
Open

use std::expected to handle burn failures? #1439

BenWibking opened this issue Jan 3, 2024 · 4 comments

Comments

@BenWibking
Copy link
Collaborator

BenWibking commented Jan 3, 2024

It would be useful to handle burn/integration failures in a more modern C++ (but exception-free) way.

This could be done by having the burner return an std::expected object (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0323r3.pdf, monadic functions: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2505r1.html)

(Public domain implementation here: https://github.com/TartanLlama/expected.)

It works like this:

enum class arithmetic_errc
{
 divide_by_zero, // 9 / 0 == ?
 not_integer_division, // 5 / 2 == 2.5 (which is not an integer)
 integer_divide_overflows, // INT_MIN / -1
};

expected<double, errc> safe_divide(double i, double j)
{
 if (j == 0) return unexpected(arithmetic_errc::divide_by_zero); // (1)
 else return i / j; // (2)
}

int divide2(int i, int j)
{
 auto e = safe_divide(i, j);
 if (!e)
 switch (e.error().value()) {
 case arithmetic_errc::divide_by_zero: return 0;
 case arithmetic_errc::not_integer_division: return i / j; // Ignore.
 case arithmetic_errc::integer_divide_overflows: return INT_MIN;
 // No default! Adding a new enum value causes a compiler warning here,
 // forcing an update of the code.
 }
 return *e;
}

In principle, std::expected objects can be propagated higher in the call stack, so it could be used thoughout Microphysics, not just in returning to the application code.

@BenWibking BenWibking changed the title use std::expected to handle burn failures use std::expected to handle burn failures? Jan 3, 2024
@maxpkatz
Copy link
Member

maxpkatz commented Jan 3, 2024

What is an example of something this enables that we can't do right now (with the burn status stored in the burn_t)?

@BenWibking
Copy link
Collaborator Author

BenWibking commented Jan 3, 2024

What is an example of something this enables that we can't do right now (with the burn status stored in the burn_t)?

It can propagate up the call chain into parts of our code that aren't Microphysics-specific. This could reduce the amount of boilerplate code significantly by using the and_then extension (https://github.com/TartanLlama/expected?tab=readme-ov-file#expected).

This extension is now part of C++23: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2505r1.html

For example, we could implement hydro retries like this:

tl::expected<image,fail_reason> attempt_hydro_update (const hydro_state& state) {
    return burn_strang_split(state)
           .and_then(hydro_stage1)
           .and_then(hydro_stage2)
           .and_then(burn_strang_split)
}

@zingale
Copy link
Member

zingale commented Jan 3, 2024

it looks like this is a C++23 feature, right?

@BenWibking
Copy link
Collaborator Author

BenWibking commented Jan 3, 2024

it looks like this is a C++23 feature, right?

Yes. You can also copy this (public domain) header file and use it now with C++17: https://github.com/TartanLlama/expected/blob/v1.1.0/include/tl/expected.hpp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants