ParmParse::addfile needs Init by ax3l · Pull Request #3440 · AMReX-Codes/amrex

ax3l · 2023-07-21T21:24:13Z

Summary

ParmParse can be used before AMReX is initialized for most functionality, which is greatly helpful for us. One function that cannot be used before init is reading from a file, because it needs an MPI context to broadcast.

Add a clean error message instead of a segfault.

Additional background

cc @EZoni @RevathiJambunathan

Checklist

The proposed changes:

fix a bug or incorrect behavior in AMReX
add new capabilities to AMReX
changes answers in the test suite to more than roundoff level
are likely to significantly affect the results of downstream AMReX users
include documentation in the code and/or rst files, if appropriate

Src/Base/AMReX_ParmParse.cpp

WeiqunZhang · 2023-07-22T01:03:22Z

This does not work because amrex::Initialized() does not work during amrex::Initialize(). I think the way to do it is

#ifdef AMREX_USE_MPI
    AMREX_ALWAYS_ASSERT_WITH_MESSAGE(ParallelDescriptor::Communicator() != MPI_COMM_NULL, "error message goes here");
#endif

ax3l · 2023-08-01T06:48:37Z

Agreed on the solution - but isn't the issue due to amrex::Error not working if AMReX is not initialized?

Src/Base/AMReX_ParmParse.cpp

WeiqunZhang · 2023-08-01T17:25:03Z

That's right. So there are two issues. We cannot call amrex::Initialized() and ParallelDescriptor::Abort() here. So maybe we can do

#ifdef AMREX_USE_MPI
    if (ParallelDescriptor::Communicator() == MPI_COMM_NULL) {
        std::cerr << "......" << std::endl;
        std::abort();
    }
#endif

ax3l · 2023-09-18T16:19:12Z

Yes, but I would rather throw an exception.
This error is not fatal and one can recover in interactive contexts if we throw.

`ParmParse` can be used before AMReX is initialized for most functionality, which is greatly helpful for us. One function that cannot be used before init is reading from a file, because it needs an MPI context to broadcast. Add a clean error message instead of a segfault.

## Summary If a file added via `ParmParse:addFile` does not exist, we did not yet receive a user-friendly error message. This fixes this in a way that does not hammer the file system from all MPI ranks. ## Additional background Follow-up to #2842 #2936 #3440 X-ref: BLAST-WarpX/warpx#5283 BLAST-ImpactX/impactx#704

ax3l added the enhancement label Jul 21, 2023

ax3l requested review from RevathiJambunathan and WeiqunZhang July 21, 2023 21:24

ax3l force-pushed the topic-amrex-addfile-init branch from aa4fd0c to 452a29b Compare July 21, 2023 21:26

ax3l commented Jul 21, 2023

View reviewed changes

Src/Base/AMReX_ParmParse.cpp Show resolved Hide resolved

ax3l commented Aug 1, 2023

View reviewed changes