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

DetectorImp::endDocument() called twice for some geometries? #1296

Open
1 task done
Zehvogel opened this issue Jul 15, 2024 · 4 comments
Open
1 task done

DetectorImp::endDocument() called twice for some geometries? #1296

Zehvogel opened this issue Jul 15, 2024 · 4 comments
Labels

Comments

@Zehvogel
Copy link
Contributor

Check duplicate issues.

  • Checked for duplicates

Goal

Initially I only wanted to understand why the OpenDataDetector has two entries per ddrec surface in the SurfaceManager maps. Turns out that the detector elements are already duplicated in the DetectorImp::m_detectorTypes map...

As far as I can tell DetectorImp::mapDetectorTypes() gets called twice which can only happen if DetectorImp::endDocument() is called twice. Naively I would expect that this should not happen?

Operating System and Version

Alma Linux 9

compiler

gcc11.4.1

ROOT Version

6.30.06

DD4hep Version

fe64884

Reproducer

source /cvmfs/sw-nightlies.hsf.org/key4hep/setup.sh -r 2024-07-15
python repro.py

with repro.py:

import os

from dd4hep import Detector

theDetector = Detector.getInstance()
theDetector.fromXML(f"{os.environ['OPENDATADETECTOR']}/xml/OpenDataDetector.xml")

# just the content of m_detectors in DetectorImp
m_detectors = [a.first for a in theDetector.detectors()]
print(f"m_detectors: {m_detectors}")

m_detectorTypes = {b:[a.name() for a  in theDetector.detectors(b, False)] for b in theDetector.detectorTypes()}
print(f"m_detectorTypes: {m_detectorTypes}")

results in:

m_detectors: ['BeamPipe', 'ECal', 'ECalBarrel', 'ECalEndcap', 'HCal', 'HCalBarrel', 'HCalEndcap', 'LongStripBarrel', 'LongStripEndcapN', 'LongStripEndcapP', 'LongStrips', 'OpenDataTracker', 'PST', 'PixelBarrel', 'PixelEndcapN', 'PixelEndcapP', 'Pixels', 'ShortStripBarrel', 'ShortStripEndcapN', 'ShortStripEndcapP', 'ShortStrips', 'Solenoid', 'world']
m_detectorTypes: {'': [], 'calorimeter': ['ECalBarrel', 'ECalEndcap', 'HCalBarrel', 'HCalEndcap', 'ECalBarrel', 'ECalEndcap', 'HCalBarrel', 'HCalEndcap'], 'compound': ['ECal', 'HCal', 'LongStrips', 'OpenDataTracker', 'Pixels', 'ShortStrips', 'ECal', 'HCal', 'LongStrips', 'OpenDataTracker', 'Pixels', 'ShortStrips'], 'passive': ['BeamPipe', 'PST', 'Solenoid', 'BeamPipe', 'PST', 'Solenoid'], 'tracker': ['LongStripBarrel', 'LongStripEndcapN', 'LongStripEndcapP', 'PixelBarrel', 'PixelEndcapN', 'PixelEndcapP', 'ShortStripBarrel', 'ShortStripEndcapN', 'ShortStripEndcapP', 'LongStripBarrel', 'LongStripEndcapN', 'LongStripEndcapP', 'PixelBarrel', 'PixelEndcapN', 'PixelEndcapP', 'ShortStripBarrel', 'ShortStripEndcapN', 'ShortStripEndcapP']}

where everything is duplicated...

Additional context

No response

@Zehvogel Zehvogel added the bug label Jul 15, 2024
@jmcarcell
Copy link
Member

jmcarcell commented Jul 22, 2024

It looks like the parser is getting confused by having all the plugins defined in a different file. As a workaround, if you copy the contents of OpenDataDetectorPlugins.xml between the <plugins> tags inside the <plugins> tag of the main xml file it won't duplicate the detectors. Looking in the examples and the detectors in k4geo I couldn't find anyone that has the plugins defined in another file like ODD, the closest is this example that I haven't been able to load with fromXML.

More precisely, the plugin file is found not to have a geometry and then DD4hep will try to close the document as it follows from this logic:

xml_elt_t compact(element);
bool steer_geometry = compact.hasChild(_U(geometry));
bool open_geometry = true;
bool close_document = true;
bool close_geometry = true;
bool build_reflections = false;
xml_dim_t world = element.child(_U(world), false);
if (element.hasChild(_U(debug)))
(Converter<Debug>(description))(xml_h(compact.child(_U(debug))));
if ( steer_geometry ) {
xml_elt_t steer = compact.child(_U(geometry));
if ( steer.hasAttr(_U(open)) )
open_geometry = steer.attr<bool>(_U(open));
if ( steer.hasAttr(_U(close)) )
close_document = steer.attr<bool>(_U(close));
if ( steer.hasAttr(_U(reflect)) )

@andresailer
Copy link
Member

Similar to #1062 ?

@MarkusFrankATcernch
Copy link
Contributor

For one single XML file and no steering of opening and closing inside the xml this cannot happen.
If you steer the closing of the geometry inside the compact XML, you can only close once: in the last file.
If you indicate closing twice in the xml, you close twice. This is what supposedly happens.

@Zehvogel
Copy link
Contributor Author

Zehvogel commented Aug 8, 2024

So is the inclusion of plugins the source? And if so will that be fixed? The reply in #1180 led me to believe that it is intended to work :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants