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

JANA2@master: duplication of std::vector<std::string> default parameters #256

Closed
wdconinc opened this issue Oct 18, 2023 · 3 comments
Closed

Comments

@wdconinc
Copy link
Member

With JANA2@master (1dc9158), the interpretation of the geometry parameter dd4hep::xml_files leads to duplication of the default value.

Steps to reproduce

  1. Compile EICrecon (I was using 8bfe438e821e4e8d7c2832b682b00e570c66b0d3) against JANA2@master
  2. Set geometry to craterlake, export DETECTOR_CONFIG=epic_craterlake
  3. Run eicrecon sim_dis_18x275_minQ2=1000_craterlake.edm4hep.root with that file from https://github.com/eic/EICrecon/actions/runs/6566489975

Expected results

This should load geometry once.

Actual results

Tries (and fails) to load geometry twice (then 4 times, then 8 times, you get the idea).

[JEventProcessorPODIO] [info] Persisting collection 'ZDCEcalClusterAssociations'
[JEventProcessorPODIO] [info] Persisting collection 'ZDCEcalClusters'
[JEventProcessorPODIO] [info] Persisting collection 'ZDCEcalRawHits'
[JEventProcessorPODIO] [info] Persisting collection 'ZDCEcalRecHits'
[JEventProcessorPODIO] [info] Persisting collection 'ZDCEcalTruthClusterAssociations'
[JEventProcessorPODIO] [info] Persisting collection 'ZDCEcalTruthClusters'
PersistencyIO    INFO  +++ Set Streamer to dd4hep::OpaqueDataBlock
Info in <TGeoManager::TGeoManager>: Geometry , Detector Geometry created
Info in <TGeoNavigator::BuildCache>: --- Maximum geometry depth set to 100
[INFO] Loading DD4hep geometry from 2 files
[INFO]   - '/opt/local/share/epic/epic_craterlake.xml'
[INFO]   - '/opt/local/share/epic/epic_craterlake.xml'
[INFO]   - loading geometry file:  '/opt/local/share/epic/epic_craterlake.xml' (patience ....)
Info in <TGeoManager::SetTopVolume>: Top volume is world_volume. Master volume is world_volume
Info in <TGeoManager::CheckGeometry>: Fixing runtime shapes...
Info in <TGeoManager::CheckGeometry>: ...Nothing to fix
Info in <TGeoManager::CloseGeometry>: Counting nodes...
Info in <TGeoManager::Voxelize>: Voxelizing...
Info in <TGeoManager::CloseGeometry>: Building cache...
Info in <TGeoManager::CountLevels>: max level = 7, max placements = 2989
Info in <TGeoManager::CloseGeometry>: 4859544 nodes/ 2895 volume UID's in Detector Geometry
Info in <TGeoManager::CloseGeometry>: ----------------modeler ready----------------
[INFO]   - loading geometry file:  '/opt/local/share/epic/epic_craterlake.xml' (patience ....)
[ERROR] Problem loading geometry: dd4hep: Attempt to add an already existing object:world_limits.
dd4hep: Error interpreting XML nodes of type <limitset/>
dd4hep: Error interpreting XML nodes of type <limits/>
dd4hep: while parsing /opt/local/share/epic/epic_craterlake.xml
[WARN] Parameter 'dd4hep:xml_files' has conflicting defaults: '/opt/local/share/epic/epic_craterlake.xml,/opt/local/share/epic/epic_craterlake.xml,/opt/local/share/epic/epic_craterlake.xml' vs '/opt/local/share/epic/epic_craterlake.xml'
Info in <TGeoManager::TGeoManager>: Geometry , Detector Geometry created
[INFO] Loading DD4hep geometry from 4 files
[INFO]   - '/opt/local/share/epic/epic_craterlake.xml'
[INFO]   - '/opt/local/share/epic/epic_craterlake.xml'
[INFO]   - '/opt/local/share/epic/epic_craterlake.xml'
[INFO]   - '/opt/local/share/epic/epic_craterlake.xml'
[INFO]   - loading geometry file:  '/opt/local/share/epic/epic_craterlake.xml' (patience ....)
Evaluator        WARN  +++ Overwriting variable: name=ElectronBeamEnergy value=5*GeV  Evaluator::Object : existing variable [setVariable error]
Evaluator        WARN  +++ Overwriting variable: name=IonBeamEnergy value=41*GeV  Evaluator::Object : existing variable [setVariable error]
Evaluator        WARN  +++ Overwriting variable: name=FieldScaleFactor value=1.0  Evaluator::Object : existing variable [setVariable error]
Evaluator        WARN  +++ Overwriting variable: name=Q1eR_Gradient value=-0.739741*tesla/meter/GeV*ElectronBeamEnergy  Evaluator::Object : existing variable [setVariable error]
Evaluator        WARN  +++ Overwriting variable: name=Q2eR_Gradient value=0.66821*tesla/meter/GeV*ElectronBeamEnergy  Evaluator::Object : existing variable [setVariable error]
Evaluator        WARN  +++ Overwriting variable: name=Q3eR_Gradient value=0.21674*tesla/meter/GeV*ElectronBeamEnergy  Evaluator::Object : existing variable [setVariable error]
Evaluator        WARN  +++ Overwriting variable: name=B2AeR_B value=0.0106667*tesla/GeV*ElectronBeamEnergy  Evaluator::Object : existing variable [setVariable error]
Evaluator        WARN  +++ Overwriting variable: name=B2BeR_B value=0.0132222*tesla/GeV*ElectronBeamEnergy  Evaluator::Object : existing variable [setVariable error]
Evaluator        WARN  +++ Overwriting variable: name=LumiSweepMag_B value=1/1.2*tesla  Evaluator::Object : existing variable [setVariable error]

Debugging

Info lines:

[INFO] Loading DD4hep geometry from 2 files
[INFO]   - '/opt/local/share/epic/epic_craterlake.xml'
[INFO]   - '/opt/local/share/epic/epic_craterlake.xml'

were added with

diff --git a/src/services/geometry/dd4hep/DD4hep_service.cc b/src/services/geometry/dd4hep/DD4hep_service.cc
index 214fcc45..1240afdf 100644
--- a/src/services/geometry/dd4hep/DD4hep_service.cc
+++ b/src/services/geometry/dd4hep/DD4hep_service.cc
@@ -106,6 +106,11 @@ void DD4hep_service::Initialize() {
     try {
         dd4hep::setPrintLevel(static_cast<dd4hep::PrintLevel>(print_level));
         LOG << "Loading DD4hep geometry from " << m_xml_files.size() << " files" << LOG_END;
+        for (auto &filename : m_xml_files) {
+            auto resolved_filename = resolveFileName(filename, detector_path_env);
+            LOG << "  - '" << resolved_filename << "'" << LOG_END;
+        }
+
         for (auto &filename : m_xml_files) {
 
             auto resolved_filename = resolveFileName(filename, detector_path_env);

Relevant JANA2 calls at https://github.com/eic/EICrecon/blob/acts-upgrade-26/src/services/geometry/dd4hep/DD4hep_service.cc#L64-L84:

    // The current recommended way of getting the XML file is to use the environment variables
    // DETECTOR_PATH and DETECTOR_CONFIG or DETECTOR(deprecated).
    // Look for those first, so we can use it for the default
    // config parameter. (see https://github.com/eic/EICrecon/issues/22)
    auto *detector_config_env = std::getenv("DETECTOR_CONFIG");
    auto *detector_path_env = std::getenv("DETECTOR_PATH");

    std::string detector_config;
    // Check if detector_config_env is set
    if(detector_config_env != nullptr) {
        detector_config = detector_config_env;
    }

    // do we have default file name
    if(!detector_config.empty()) {
        m_xml_files.push_back(std::string(detector_path_env ? detector_path_env : ".") + "/" + detector_config + ".xml");
    }

    // User may specify multiple geometry files via the config. parameter. Normally, this
    // will be a single file which itself has includes for other files.
    app->SetDefaultParameter("dd4hep:xml_files", m_xml_files, "Comma separated list of XML files describing the DD4hep geometry. (Defaults to ${DETECTOR_PATH}/${DETECTOR_CONFIG}.xml using envars.)");

Passing -Pdd4hep:xml_files=$DETECTOR_PATH/$DETECTOR_CONFIG.xml works fine; it's just the defaults that are duplicated on parsing.

@wdconinc
Copy link
Member Author

Hi @AyanRoy16 , I haven't looked through the recent parameter handling fixes you implemented, but that's where I would start. I don't think EICrecon does anything strange, so I expect this can be reproduced in a unit test.

@wdconinc
Copy link
Member Author

#244 is the one I'd bisect around (if I had the time)

@nathanwbrei
Copy link
Collaborator

Addressed in PR #257

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

No branches or pull requests

2 participants