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

[WIP] MultiFabRegister: throw in get #5356

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Oct 2, 2024

Close #5319
Follow-up to #5230

  • Throw a runtime exception instead of returning a nullptr if a field is requested via the getter.
  • update logic to ensure this passes all tests

@ax3l ax3l added cleaning Clean code, improve readability component: ABLASTR components shared with other PIC codes labels Oct 2, 2024
@ax3l ax3l requested a review from EZoni October 2, 2024 15:54
Throw a runtime exception instead of returning a `nullptr` if a
field is requested via the getter.
@ax3l ax3l requested a review from lucafedeli88 October 2, 2024 16:19
@ax3l ax3l changed the title MultiFabRegister: throw in get [WIP] MultiFabRegister: throw in get Oct 2, 2024
@ax3l ax3l added the help wanted Extra attention is needed label Oct 2, 2024
@EZoni
Copy link
Member

EZoni commented Oct 18, 2024

Discussed with @RemiLehe offline:

The functions get_mr_levels and get_mr_levels_alldirs fails by definition when we call it on coarse-patch MultiFabs, e.g., current_cp, because those are allocated only on levels greater than level 0 while the function attempts to return all levels starting from 0.

We will try to add to get_mr_levels and get_mr_levels_alldirs a function argument, e.g., skip_level_0, such that the function returns a nullptr for level 0 instead of calling the internal functions of the MultiFabRegister and throwing an exception.

Comment on lines +353 to 354
throw std::runtime_error("MultiFabRegister::get name does not exist in register: " + internal_name);
return nullptr;
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove this:

Suggested change
throw std::runtime_error("MultiFabRegister::get name does not exist in register: " + internal_name);
return nullptr;
throw std::runtime_error("MultiFabRegister::get name does not exist in register: " + internal_name);

Comment on lines +367 to 368
throw std::runtime_error("MultiFabRegister::get name does not exist in register: " + internal_name);
return nullptr;
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove this:

Suggested change
throw std::runtime_error("MultiFabRegister::get name does not exist in register: " + internal_name);
return nullptr;
throw std::runtime_error("MultiFabRegister::get name does not exist in register: " + internal_name);

@EZoni EZoni self-requested a review October 19, 2024 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleaning Clean code, improve readability component: ABLASTR components shared with other PIC codes help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Throw error in MultiFabRegister.get when a field does not exist
2 participants