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 external assert library #172

Merged
merged 7 commits into from
Jan 15, 2025
Merged

Conversation

bonachea
Copy link
Member

@bonachea bonachea commented Jan 13, 2025

This PR replaces Caffeine's internal use of the stub prif:prif_private_s:caffeine_assert_s submodule with the external berkeleylab/assert library.

The main benefits of this upgrade:

  1. Macro-flavored assertions that are guaranteed to compile away to zero runtime overhead when assertions are disabled, regardless of compiler analysis
  2. Automatic file/line reporting to the console for failed assertions
  3. Broader type support for assert diagnostic data, including characterizable_t and intrinsic_array_t
  4. Leverage GASNet's automatic backtracing support and debugger integrations for assertion failures on any platform.

Here is example fpm test output from a manually injected assertion failure:

A total of 59 test cases

*** FATAL ERROR (proc 2): Assertion "prif_co_min: LHS/RHS length match in file ././src/caffeine/collective_subroutines/co_min_s.F90, line 30" failed on image 3 with diagnostic data "we        , like     "
*** NOTICE (proc 2): Before reporting bugs, run with GASNET_BACKTRACE=1 in the environment to generate a backtrace.
*** Caught a fatal signal (proc 2): SIGABRT(6)
<ERROR> Execution for object " caffeine-test " returned exit code  134
<ERROR> *cmd_run*:stopping due to failed executions
STOP 134

This PR notably utilizes the ASSERT_PARALLEL_CALLBACKS feature added in assert library version 2.0.1, which was added specifically for Caffeine's use case. This ensures:

  1. That a failing assertion reports the Caffeine-provided image number, rather than natively invoking the THIS_IMAGE() parallel feature.
  2. That a failing assertion passes control back to Caffeine to handle job teardown, rather than natively invoking the ERROR STOP parallel feature.

Both of these are intended to break a dependency cycle, ensuring Caffeine's dependency on the assert library does not complicate builds for the compiler client by utilizing native parallel features, or result in an explicitly circular dependency back to the prif module.

Finally, this PR enables control of assertion enforcement on the install.sh command line, eg:

env CPPFLAGS=-DASSERTIONS=0 install.sh ...

the default remains assertions enabled while Caffeine development is still in a pre-production state.

@bonachea bonachea marked this pull request as ready for review January 13, 2025 21:46
@bonachea bonachea requested a review from rouson January 13, 2025 21:47
This uses gasnett_fatalerror for process termination, for two reasosn:

1. prif_error_stop is impure and thus cannot be invoked by assert_error_stop
2. gasnett_fatalerror provides a variety of distributed-aware debugging services,
   such as automated backtracing and debugger attach points
This is a filthy hack that should be replaced with a user-controllable option
This ensures we get multi-image assertion capabilities, even for
compilers that normally default to single-image (flang-new)
This enables command-line control of assertions, eg:

```
env CPPFLAGS=-DASSERTIONS=0 install.sh ...
```
Copy link
Collaborator

@rouson rouson left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks, @bonachea!

@bonachea bonachea merged commit 3a5e5a6 into BerkeleyLab:main Jan 15, 2025
3 checks passed
bonachea added a commit to bonachea/caffeine that referenced this pull request Jan 17, 2025
This is a left-over from pull request BerkeleyLab#171 and BerkeleyLab#172 that was only
possible to fully resolve now they are both merged.
@bonachea bonachea mentioned this pull request Jan 17, 2025
bonachea added a commit to bonachea/caffeine that referenced this pull request Jan 17, 2025
This is a left-over from pull request BerkeleyLab#171 and BerkeleyLab#172 that was only
possible to fully resolve now they are both merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants