Restore ability to run WCSim interactively

[spradlin]

          ### G4UI_USE

The preprocessor flag G4UI_USE is used in the top-level main source file WCSim.cc. The lines of code optionally included by setting the flag appear on L20-L22 and L136-L144. It looks like this is intended to allow WCSim to be run with an interactive UI. This is an excellent candidate for something to be steered by CMake.

However, in its current state, this cannot work. Note that the optionally included code that would launch an interactive session is all in an if-block, L129-L150. The condition for executing the interactive session is that there are no command line arguments. However, L47-L53 will terminate execution if the number of command line arguments is not exactly 2.

Hmm, this is another thing that should probably be spun off into its own issue.

Originally posted by @spradlin in https://github.com/spradlin/WCSim/issues/17#issuecomment-1382080747

[spradlin]

Modes of operation

WCSim/develop

The 'old' standard WCSim/develop:WCSim.cc appears to define two modes of operation:

1. No commnd line arguments
2. One or more command line arguments

Mode of operation 1 depends on whether WCSim has been built to support an interactive interface:

• If yes, then WCSim will start an interactive session,
• If no, then WCSim will clean up and exit without running simulation.

Mode of operation 2 is a batch mode. WCSim will treat the first command line argument as a GEANT4 macro. It will execute the macro and then exit. If more than one command line argument is supplied, the additional arguments are ignored.

tdealtry/main_into_hybrid

Although the control logic is more complex in tdealtry/main_into_hybrid:WCSim.cc, functionally it defines exactly one mode of operation. This is a batch mode that requires exactly two command line arguments with an assumed ordering:

1. The main execution macro (equivalent to the first argument the WCSim/develop mode 2),
2. A macro that defines tuning parameters.

As noted in the description of this issue, there is no functional interactive mode.

Comments about batch modes

I am not happy with either of the two batch mode implementations.

The WCSim/develop version allows an arbitrary number of command line arguments but only uses the first. This is very unintuitive behavior. Two strategies that make more sense to me:

1. Exit with an error message if more than one argument is supplied (similar to the strategy adopted in tdealtry/main_into_hybrid),
2. Process all of the command line arguments in order.

I find the second strategy very attractive.

• It is consistent with the usage of common UNIX utilities and with behavior of many other physics applications with which i have worked,
• It allows more modularity of configuration and steering macros. (There may be good reasons to avoid this kind of modularity. It can lead to unexpected parameter conflicts).

In the tdealtry/main_into_hybrid version, the hard-coded need for a tuning macro command line argument does not make sense to me.

• The tuning parameters should have reasonable defaults! These defaults should be used unless overridden by commands,
• The tuning macro is the second argument, but it is processed first. This behavior would make sense if the tuning parameter macro were optional, but it doesn't make sense to me for a mandatory command line argument. (This is a subjective complaint).

So, i would like a redesign of the batch mode operation. I don't know how the suggestion to do so would be received....

[spradlin]

Special place of tuning parameter macros

Despite my complaints in the previous comment, the addition of a dedicated command line argument for the tuning parameter macro is an improvement over the hard-coded filename for tuning parameters in WCSim/develop. It allows different collections of tuning parameters to be maintained and used.

However, i do not yet understand why they need special treatment. Why are they not executed from a top-level job macro, such as WCSim.mac?

First!

According to a comment in include/WCSimTuningParameters.hh, the WCSimTuningParameters class contains

  // The parameters that need to be set before WCSimDetectorConstruction
  // is created

Before the creation of the WCSimDetectorConstruction object. The usage in WCSim.cc reflects this statement:

    45    WCSimTuningParameters* tuningpars = new WCSimTuningParameters();
    :
    54    // Get the tuning parameters
    55    //hack B.Q
    56    G4cout << "B.Q: Read argv[2], which should contains the tuning parameters" << G4endl;
    57    G4String fileName2 = argv[2];
    58    file_exists(fileName2);
    59    if(fileName2 == "vis.mac"){
    60      G4cout << "ERROR: Execute without arg for interactive mode" << G4endl;
    61      //return -1;
    62    }
    63    G4String command2 = "/control/execute ";
    64    UI->ApplyCommand(command2+fileName2);
    :
    71    // UserInitialization classes (mandatory)
    72    enum DetConfiguration {wfm=1,fwm=2};
    73    G4int WCSimConfiguration = fwm;
    74
    75    WCSimDetectorConstruction* WCSimdetector = new
    76      WCSimDetectorConstruction(WCSimConfiguration,tuningpars);

Why are these parameters required at creation? The whole detector geometry can be revised and redefined after the creation of WCSimDetectorConstruction. Why are these parameters special?

WCSimDetectorConstruction usage of WCSimTuningParameters

What does the WCSimDetectorConstruction constructor do with its pointer to a WCSimTuningParameters?

• L41: It makes a copy of the pointer that is stored as the data member WCSimDetectorConstruction::WCSimTuningParams. This is good. It means that post-construction changes to the tuning parameters can be accessed, in principle.

The copied pointer WCSimDetectorConstruction::WCSimTuningParams has a corresponding accessor WCSimDetectorConstruction::GetParameters().

In the constructor, there are no direct calls to the original parameter, the copied pointer, nor the accessor to the copied pointer. If the final values for the WCSimTuningParameters are needed by the constructor, it must be indirectly.

All uses of the tuning parameters by WCSimDetectorConstruction appear to go through the WCSimDetectorConstruction::WCSimTuningParams copy of the pointer. Performing a grep for WCSimTuningParams finds the following instances

# grep WCSimTuningParams src/*.cc
src/WCSimConstructCylinder.cc:  const G4bool WCTopVeto = (WCSimTuningParams->GetTopVeto());
src/WCSimConstructCylinder.cc:  G4double WCTVPMTSpacing = (WCSimTuningParams->GetTVSpacing())*cm;
src/WCSimConstructMaterials.cc:  double WCODWLSCladdingReflectivity = WCSimTuningParams->GetWCODWLSCladdingReflectivity();
src/WCSimConstructMaterials.cc:    ABWFF = WCSimTuningParams->GetAbwff();
src/WCSimConstructMaterials.cc:   RAYFF = WCSimTuningParams->GetRayff();
src/WCSimConstructMaterials.cc:   G4double MIEFF = WCSimTuningParams->GetMieff();
src/WCSimConstructMaterials.cc:   BSRFF = WCSimTuningParams->GetBsrff();
src/WCSimConstructMaterials.cc:   RGCFF = WCSimTuningParams->GetRgcff();   //defaults in mac: 0.32 and flat
src/WCSimConstructMaterials.cc:  double WCODTyvekReflectivity = WCSimTuningParams->GetWCODTyvekReflectivity();
src/WCSimDetectorConstruction.cc:  WCSimTuningParams(WCSimTuningPars),
src/WCSimPMTQE.cc:  QEFF = WCSimTuningParams->GetQeff();   //defaults in mac: 1.

in addition to its ubiquitous usage in the WCSimTuningMessenger class. There appears to be exactly one call to the accessor, and that is from the WCSimWCDigitizerSKI::DigitizeHits() method. This function should be called by the run manager for each event---not needed in the construction of any objects.

Aside: with regards to WCSimWCDigitizerSKI::DigitizeHits() getting a digitizer saturation parameter through the detector object WCSimDetectorConstruction: Why? Why?

Both of the uses of the WCSimDetectorConstruction::WCSimTuningParams pointer in src/WCSimConstructCylinder.cc, L714 and L884, are called in the WCSimDetectorConstruction::ConstructCylinder() function. This function, in turn, i called in only one place in WCSimDetectorConstruction::Construct(). And in turn, this function seems to be called only by WCSimDetectorConstruction::UpdateGeometry(). And this is only called by the void WCSimDetectorMessenger::SetNewValue() macro message processor when the /WCSim/Construct macro command is invoked.

Aside: This trace seems incomplete. I know that the SuperK detector geometry is always constructed, and then, if the detector configuration is changed, then it is replaced by the desired geometry. I do not see that initial construction of the SuperK geometry in this trace.

Back to the uses of WCSimDetectorConstruction::WCSimTuningParams, the instances in src/WCSimConstructMaterials.cc are all in the function WCSimDetectorConstruction::ConstructMaterials(). This is called by the WCSimDetectorConstruction::WCSimDetectorConstruction() constructor.

Okay, this is why the tuning parameters need to be processed before the WCSimDetectorConstruction::WCSimDetectorConstruction() construtor is invoked. It is because the constructor defines the G4 materials table and there is no facility to update the materials after the constructor is invoked. I think that can be fixed.

[spradlin]

These issues ideally should be addressed before restoring interactive UI functionality. It would be possible to proceed at the top level if the WCSimTuningParameters object is initialized with reasonable values at construction. This would allow us to launch an interactive session without any command line arguments.

However, any interactive session running in such a way would be stuck with those defaults. We would still need a way to update the tuning parameters after calling the WCSimDetectorConstruction constructor if we wanted to be able to interactively update the tuning parameters.

The benefit of the interactive UI to geometry debugging seems more immediate than its more general usability. I am not entirely happy with rehacking a hack, but it makes sense at the moment.

[spradlin]

Default values of WCSimTuningParameters

The WCSimTuningParameters::WCSimTuningParameters() constructor assigns a number of default values to its data members. Just check that for each data member in the class declaration, there is a default value assigned. Also, compare the values to those in tuningNominal.mac.

Par	default	tuning macro
rayff	0.75	0.75
bsrff	2.50	2.5
abwff	1.30	1.30
rgcff	0.32	0.32
qeff	1.0	-
mieff	0.0	0.0
ttsff	1.0	-
pmtsatur	-1	-
tvspacing	100.0	100
topveto	false	0
WCODWLSCladdingReflectivity	0.90	-
WCODTyvekReflectivity	0.90	-

OK, every parameter has a default value. And for each parameter set by tuningNominal.mac, the default value in WCSimTuningParameters::WCSimTuningParameters() is identical to the set value.

[spradlin]

Fragility of processing a .mac file before all messeners are setup

The current situation with the tuning parameters requires the tuning parameter macro to be processed after the tuning parameter messener is setup up but before all of the other WCSim intercom messeners. This is bad!.

I was hoping that the UI manager might hold commands if it did not find an messenger available messenger. However, this is not the case. To test my hoped-for behavior, i moved the creation of WCSimTuningParameters to below the processing of the tuning macro. This resulted in a run-time error message:

  :
B.Q: Read argv[2], which should contains the tuning parameters
***** COMMAND NOT FOUND </WCSim/tuning/abwff 1.29> *****

***** Batch is interrupted!! *****
  :

Making this robust will require a resolution to #21

[spradlin]

Proposed implementation

Unfortunately, it looks like there must be at least two conditionally compiled blocks in the body of main() based on whether G4UI_USE is defined:

1. One at the beginning to determine how to deal with absence of command-line arguments (fail or run interactively)
2. One after all of the creation and setup wrap the launch of interactive UI (e.g., L129-L150 in current version)

I would like to make these blocks self-contained, without #else clauses, if possible. Just compact drop-in code.

Command line argument treatment

So, for now, will preserve what appears to be the intended modes of operation based on the number of command line arguments:

• If not built to support interactive UI:
  • 2 args (argc==3): batch mode: process tuning macro and continue setup
  • any other number: undefined: abort
• If built to support interactive UI:
  • 0 args: interactive mode: continue with setup
  • 2 args (argc==3): batch mode: process tuning macro and continue setup
  • any other number: undefined: abort

With my goal of making the optionally compiled code in compact drop-in blocks, i think that a switch-case suits this behavior. Something like the following pattern:

  switch(argc) {
#ifdef G4UI_USE
    case 1 :    // Interactive mode with no command-line arguments
      // If anything needs to be done here...
      break;
#endif

    case 3 :    // Batch mode with two command-line arguments
      // Check presence of input files
      // Process tuning parameters
      break;

    // Otherwise proceed to default exit condition
    default:    // Unrecognized number of valid command line arguments
      // Print usage statement
      // Clean up
      // Exit.
  }

In this way, the no-argument interactive mode will be included if and only if WCSim is built with G4UI_USE.

Executing user input

After all of the WCSim components are created and the run manager is initialized (after L127 in the current version), then the user input needs to be executed. Either the job macro is executed, if running in batch mode, or the interactive UI should be launched if the job is built to suppor that and if the number of command line arguments indicate that it should.

I wanted to make the running mode more explicit. So, i introduced an enumeration class and created a variable of that class to track the execution mode:

enum class WCSimExeMode {Batch, Interactive, Unknown};
  :
int main(int argc,char** argv)
{
  WCSimExeMode exemode = WCSimExeMode::Unknown;
  :
}

Then i set the value of the exemode variable in the switch-case where the execution mode is deduced from the command line arguments.

Now i can use the value of exemode at the end to control the processing of the user input in another switch-case like

  switch(exemode) {
    case WCSimExeMode::Batch : {
      // Run the job macro
      break;
    }

    case WCSimExeMode::Interactive : {
#ifdef G4UI_USE
      // Launch the interactive UI
#endif
      break;
    }

    case WCSimExeMode::Unknown : {
      G4cerr << "SOMETHING IS WRONG.  Attempt to execute in undefined mode." << G4endl;
    }
  }

This seems to work as intended!