Fix GeantSimpleCalo output when Celeritas is enabled (#1040)

* Don't create ROOT file if not using ROOT SD
* Improve debug messages
* Synchronize logger output
* Add hack to fix overwriting of output registry
* Print exception code when about to raise (in case another thread is still running)
* Fix crash on exit due to GUI manager

app/celer-g4/ActionInitialization.cc

@@ -84,7 +84,7 @@ void ActionInitialization::BuildForMaster() const
  */
 void ActionInitialization::Build() const
 {
-    CELER_LOG_LOCAL(status) << "Constructing user actions on worker threads";
+    CELER_LOG_LOCAL(status) << "Constructing user action";
 
     // Primary generator emits source particles
     if (hepmc_gen_)

app/celer-g4/DetectorConstruction.cc

@@ -262,9 +262,10 @@ void DetectorConstruction::ConstructSDandField()
     }
     else if (sd_type == SensitiveDetectorType::simple_calo)
     {
-        CELER_LOG_LOCAL(status) << "Attaching simple calorimeters";
         for (auto& calo : simple_calos_)
         {
+            CELER_LOG_LOCAL(status)
+                << "Attaching simple calorimeter '" << calo->label() << '\'';
             // Create and attach SD
             auto detector = calo->MakeSensitiveDetector();
             CELER_ASSERT(detector);
@@ -292,7 +293,7 @@ void DetectorConstruction::ConstructSDandField()
             sd_manager->AddNewDetector(detector.release());
 
             // Add to ROOT output
-            if (RootIO::use_root())
+            if (GlobalSetup::Instance()->root_sd_io())
             {
                 RootIO::Instance()->AddSensitiveDetector(start->first);
             }

app/celer-g4/EventAction.cc

@@ -70,7 +70,7 @@ void EventAction::EndOfEventAction(G4Event const* event)
         CELER_TRY_HANDLE(transport_->Flush(), call_g4exception);
     }
 
-    if (RootIO::use_root())
+    if (GlobalSetup::Instance()->root_sd_io())
     {
         // Write sensitive hits
         RootIO::Instance()->Write(event);

app/celer-g4/ExceptionHandler.cc

@@ -68,7 +68,7 @@ G4bool ExceptionHandler::Notify(char const* origin_of_exception,
                 }
                 else
                 {
-                    CELER_LOG_LOCAL(critical) << "Aborting run due to exception";
+                    CELER_LOG_LOCAL(critical) << "Aborting run due to exception (" << exception_code << ")";
                     run_man->AbortRun();
                 }
             }

app/celer-g4/GeantDiagnostics.cc

@@ -77,6 +77,10 @@ GeantDiagnostics::GeantDiagnostics(SharedParams const& params)
             celeritas::environment()));
 #endif
         output_reg->insert(std::make_shared<BuildOutput>());
+
+        // Save filename from global options (TODO: remove this hack)
+        const_cast<SharedParams&>(params).set_output_filename(
+            global_setup.setup_options().output_file);
     }
 
 #if CELERITAS_USE_JSON

app/celer-g4/GlobalSetup.cc

@@ -16,6 +16,7 @@
 #include "corecel/io/Logger.hh"
 #include "corecel/io/StringUtils.hh"
 #include "corecel/sys/Device.hh"
+#include "celeritas/ext/RootFileManager.hh"
 #include "celeritas/field/RZMapFieldInput.hh"
 #include "accel/ExceptionConverter.hh"
 #include "accel/SetupOptionsMessenger.hh"
@@ -171,11 +172,18 @@ void GlobalSetup::ReadInput(std::string const& filename)
         options_->output_file = input_.output_file;
     }
 
-    if (input_.sd_type == SensitiveDetectorType::event_hit
-        && !RootIO::use_root())
+    if (input_.sd_type == SensitiveDetectorType::event_hit)
     {
-        CELER_LOG(warning) << "Collecting SD hit data that will not be "
-                              "written because ROOT is disabled";
+        root_sd_io_ = RootFileManager::use_root();
+        if (!root_sd_io_)
+        {
+            CELER_LOG(warning) << "Collecting SD hit data that will not be "
+                                  "written because ROOT is disabled";
+        }
+    }
+    else
+    {
+        root_sd_io_ = false;
     }
 
     // Start the timer for setup time

app/celer-g4/GlobalSetup.hh

@@ -82,9 +82,15 @@ class GlobalSetup
 
     //// NEW INTERFACE ////
 
+    //! Get setup options
+    SetupOptions const& setup_options() const { return *options_; }
+
     //! Get user input options
     RunInput const& input() const { return input_; }
 
+    //! Whether ROOT I/O for SDs is enabled
+    bool root_sd_io() const { return root_sd_io_; }
+
   private:
     // Private constructor since we're a singleton
     GlobalSetup();
@@ -94,6 +100,7 @@ class GlobalSetup
     std::shared_ptr<SetupOptions> options_;
     RunInput input_;
     Stopwatch get_setup_time_;
+    bool root_sd_io_{false};
 
     std::unique_ptr<G4GenericMessenger> messenger_;
 };

app/celer-g4/LocalLogger.cc

@@ -7,6 +7,7 @@
 //---------------------------------------------------------------------------//
 #include "LocalLogger.hh"
 
+#include <mutex>
 #include <G4Threading.hh>
 
 #include "corecel/io/ColorUtils.hh"
@@ -21,29 +22,37 @@ namespace app
  */
 void LocalLogger::operator()(Provenance prov, LogLevel lev, std::string msg)
 {
+    // Write preamble to a buffer first
+    std::ostringstream os;
+
     int local_thread = G4Threading::G4GetThreadId();
-    std::clog << color_code('W') << '[';
+    os << color_code('W') << '[';
     if (local_thread >= 0)
     {
-        std::clog << local_thread + 1;
+        os << local_thread + 1;
     }
     else
     {
-        std::clog << 'M';
+        os << 'M';
     }
-    std::clog << '/' << num_threads_ << "] " << color_code(' ');
+    os << '/' << num_threads_ << "] " << color_code(' ');
 
     if (lev == LogLevel::debug || lev >= LogLevel::warning)
     {
         // Output problem line/file for debugging or high level
-        std::clog << color_code('x') << prov.file;
+        os << color_code('x') << prov.file;
         if (prov.line)
-            std::clog << ':' << prov.line;
-        std::clog << color_code(' ') << ": ";
+            os << ':' << prov.line;
+        os << color_code(' ') << ": ";
     }
+    os << to_color_code(lev) << to_cstring(lev) << ": " << color_code(' ');
 
-    std::clog << to_color_code(lev) << to_cstring(lev) << ": "
-              << color_code(' ') << msg << std::endl;
+    {
+        // Write buffered content and message with a mutex, then flush
+        static std::mutex clog_mutex;
+        std::lock_guard scoped_lock{clog_mutex};
+        std::clog << os.str() << msg << std::endl;
+    }
 }
 
 //---------------------------------------------------------------------------//

app/celer-g4/RootIO.cc

@@ -32,18 +32,6 @@ namespace celeritas
 {
 namespace app
 {
-//---------------------------------------------------------------------------//
-/*!
- * Whether ROOT interfacing is enabled.
- *
- * This is true unless the \c CELER_DISABLE_ROOT environment variable is
- * set to a non-empty value.
- */
-bool RootIO::use_root()
-{
-    return RootFileManager::use_root();
-}
-
 //---------------------------------------------------------------------------//
 /*!
  * Create a ROOT output file for each worker thread in MT.

app/celer-g4/RootIO.hh

@@ -35,14 +35,6 @@ class RootIO
     friend class G4ThreadLocalSingleton<RootIO>;
 
   public:
-#if CELERITAS_USE_ROOT
-    // Whether ROOT output is enabled
-    static bool use_root();
-#else
-    // ROOT is never enabled if ROOT isn't available
-    constexpr static bool use_root() { return false; }
-#endif
-
     // Return non-owning pointer to a singleton
     static RootIO* Instance();
 

app/celer-g4/RunAction.cc

@@ -68,7 +68,11 @@ void RunAction::BeginOfRunAction(G4Run const* run)
         {
             // This worker (or master thread) is responsible for initializing
             // celeritas: initialize shared data and setup GPU on all threads
-            CELER_TRY_HANDLE(params_->Initialize(*options_), call_g4exception);
+            // TODO: reusing the existing output registry is a hack needed to
+            // preserve the GeantSimpleCalo output. This will be fixed in 0.5
+            CELER_TRY_HANDLE(
+                params_->Initialize(*options_, params_->output_reg()),
+                call_g4exception);
             CELER_ASSERT(*params_);
         }
         else
@@ -115,7 +119,7 @@ void RunAction::EndOfRunAction(G4Run const*)
 {
     ExceptionConverter call_g4exception{"celer0005"};
 
-    if (RootIO::use_root())
+    if (GlobalSetup::Instance()->root_sd_io())
     {
         // Close ROOT output of sensitive hits
         CELER_TRY_HANDLE(RootIO::Instance()->Close(), call_g4exception);

src/accel/GeantSimpleCalo.cc

@@ -7,6 +7,7 @@
 //---------------------------------------------------------------------------//
 #include "GeantSimpleCalo.hh"
 
+#include "celeritas_config.h"
 #include "corecel/cont/Range.hh"
 #include "corecel/io/Logger.hh"
 #include "celeritas/ext/GeantGeoParams.hh"
@@ -16,7 +17,6 @@
 #include "SharedParams.hh"
 #include "detail/GeantSimpleCaloSD.hh"
 #include "detail/GeantSimpleCaloStorage.hh"
-
 #if CELERITAS_USE_JSON
 #    include <nlohmann/json.hpp>
 

src/accel/SharedParams.cc

@@ -18,7 +18,6 @@
 #include <G4ParticleTable.hh>
 #include <G4RunManager.hh>
 #include <G4Threading.hh>
-#include <G4UImanager.hh>
 
 #include "celeritas_config.h"
 #include "corecel/Assert.hh"
@@ -173,17 +172,18 @@ bool SharedParams::CeleritasDisabled()
 
 //---------------------------------------------------------------------------//
 /*!
- * Set up Celeritas using Geant4 data.
+ * Set up Celeritas using Geant4 data and existing output registery.
  *
- * This is a separate step from construction because it has to happen at the
- * beginning of the run, not when user classes are created. It should be called
- * from the "master" thread (for MT mode) or from the main thread (for Serial),
- * and it must complete before any worker thread tries to access the shared
- * data.
+ * A design oversight in the \c GeantSimpleCalo means that the action registry
+ * *must* be created before \c SharedParams is initialized, and in the case
+ * where Celeritas is not disabled, initialization clears the existing
+ * registry. This prevents the calorimeter from writing output.
  */
-SharedParams::SharedParams(SetupOptions const& options)
+SharedParams::SharedParams(SetupOptions const& options, SPOutputRegistry oreg)
 {
     CELER_EXPECT(!*this);
+    output_reg_ = std::move(oreg);
+
     CELER_VALIDATE(!CeleritasDisabled(),
                    << "Celeritas shared params cannot be initialized when "
                       "Celeritas offloading is disabled via "
@@ -240,6 +240,46 @@ SharedParams::SharedParams(SetupOptions const& options)
     CELER_ENSURE(*this);
 }
 
+//---------------------------------------------------------------------------//
+/*!
+ * Initialize shared data with existing output registry.
+ *
+ * TODO: this is a hack to be deleted in v0.5.
+ */
+void SharedParams::Initialize(SetupOptions const& options, SPOutputRegistry reg)
+{
+    CELER_EXPECT(reg);
+    *this = SharedParams(options, std::move(reg));
+}
+
+//---------------------------------------------------------------------------//
+/*!
+ * Save a diagnostic output filename from a Geant4 app when Celeritas is off.
+ *
+ * This will be overwritten when calling Initialized with setup options.
+ *
+ * TODO: this hack should be deleted in v0.5.
+ */
+void SharedParams::set_output_filename(std::string const& filename)
+{
+    output_filename_ = filename;
+}
+
+//---------------------------------------------------------------------------//
+/*!
+ * Set up Celeritas using Geant4 data.
+ *
+ * This is a separate step from construction because it has to happen at the
+ * beginning of the run, not when user classes are created. It should be called
+ * from the "master" thread (for MT mode) or from the main thread (for Serial),
+ * and it must complete before any worker thread tries to access the shared
+ * data.
+ */
+SharedParams::SharedParams(SetupOptions const& options)
+    : SharedParams(options, nullptr)
+{
+}
+
 //---------------------------------------------------------------------------//
 /*!
  * On worker threads, set up data with thread storage duration.
@@ -395,13 +435,8 @@ void SharedParams::initialize_core(SetupOptions const& options)
     CoreParams::Input params;
 
     // Create registries
-    if (!output_reg_)
-    {
-        output_reg_ = std::make_shared<OutputRegistry>();
-    }
-
     params.action_reg = std::make_shared<ActionRegistry>();
-    params.output_reg = output_reg_;
+    params.output_reg = this->output_reg();
 
     // Load geometry
     params.geometry = [&options] {
@@ -571,17 +606,8 @@ void SharedParams::try_output() const
     if (CELERITAS_USE_JSON && !params_ && filename.empty())
     {
         // Setup was not called but JSON is available: make a default filename
-        G4UImanager* ui = G4UImanager::GetUIpointer();
-        filename = ui->GetCurrentValues("/celer/outputFile");
-        if (!filename.empty())
-        {
-            CELER_LOG(debug) << "Set Celeritas output filename from G4UI";
-        }
-        else
-        {
-            filename = "celeritas.json";
-            CELER_LOG(debug) << "Set default Celeritas output filename";
-        }
+        filename = "celeritas.json";
+        CELER_LOG(debug) << "Set default Celeritas output filename";
     }
 
     if (filename.empty())

src/accel/SharedParams.hh

@@ -116,6 +116,17 @@ class SharedParams
 
     // Geant geometry wrapper, lazily created
     SPConstGeantGeoParams const& geant_geo_params() const;
+
+    // NASTY HACK TO BE DELETED:
+    // Construct Celeritas using Geant4 data and existing output registry
+    SharedParams(SetupOptions const& options, SPOutputRegistry reg);
+
+    // Initialize shared data on the "master" thread with existing output
+    // registry
+    void Initialize(SetupOptions const& options, SPOutputRegistry reg);
+
+    // Set the output filename when celeritas is disabled
+    void set_output_filename(std::string const&);
     //!@}
 
   private: