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

Build SIM geometry from DB #34206

Closed
civanch opened this issue Jun 22, 2021 · 17 comments
Closed

Build SIM geometry from DB #34206

civanch opened this issue Jun 22, 2021 · 17 comments

Comments

@civanch
Copy link
Contributor

civanch commented Jun 22, 2021

It is necessary to establish build of SIM geometry from DB without use of DDD components and DB records, which are obsolete. Start of the discussion was in #34167

@cmsbuild
Copy link
Contributor

A new Issue was created by @civanch Vladimir Ivantchenko.

@Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@civanch
Copy link
Contributor Author

civanch commented Jun 22, 2021

assign geometry

@cmsbuild
Copy link
Contributor

New categories assigned: geometry

@Dr15Jones,@cvuosalo,@civanch,@ianna,@mdhildreth,@makortel you have been requested to review this Pull request/Issue and eventually sign? Thanks

@civanch
Copy link
Contributor Author

civanch commented Jun 22, 2021

assign core

@cmsbuild
Copy link
Contributor

New categories assigned: core

@Dr15Jones,@smuzaffar,@makortel you have been requested to review this Pull request/Issue and eventually sign? Thanks

@makortel
Copy link
Contributor

@cvuosalo, replying here to #34167 (comment). Thanks for trying out. After further thought I think the following would be (also conceptually) a better version of #34167 (comment)

                    if self.geometryDBLabel:
                        self.executeAndRemember('if hasattr(process, "XMLFromDBSource): process.XMLFromDBSource.label="%s"'%(self.geometryDBLabel))
                        self.executeAndRemember('if hasattr(process, "DDDetectorESProducerFromDB"): process.DDDetectorESProducerFromDB.label="%s"'%(self.geometryDBLabel))

@cvuosalo
Copy link
Contributor

@makortel Thanks, your new version works, at least so far. There is more to be fixed (see next comment).

@cvuosalo
Copy link
Contributor

Request for advice: It looks to me like Configuration/Geometry/python/GeometryDB_cff.py needs to be changed to include DDD and DD4hep options. Currently, it contains:

from Configuration.Geometry.GeometryRecoDB_cff import *
from Configuration.Geometry.GeometrySimDB_cff import *

Those cff files currently contain DDD-specific code.

I think GeometryDB_cff.py should contain a DDD section and a DD4hep section. In pseudo-code, it might look something like this:

from Configuration.ProcessModifiers.dd4hep_cff import dd4hep

# DDD only
from Configuration.Geometry.GeometryRecoDB_cff import *
from Configuration.Geometry.GeometrySimDB_cff import *
# DD4hep only
from Configuration.Geometry.GeometryRecoDBDD4hep_cff import *
from Configuration.Geometry.GeometrySimDBDD4hep_cff import *

I think that the dd4hep.toReplaceWith syntax could be used to make this choice. Is GeometryDB_cff.py the right location to branch between DDD and DD4hep configs?

@makortel
Copy link
Contributor

The dd4hep.toReplaceWith() should be used for cases where the meaning of a module label is re-defined with another module of the same type (e.g. replace ESProducer with a different ESProducer). The replacement should be done in the (cfi) file that defines the module.

Different sets of cfi/cff files can be loaded into the Process along what I described in #34167 (comment). Even then I'd recommend to load as small configuration fragments at a time as possible (e.g. I'm a bit concerned that full GeometryRecoDB_cff could be too complex). In the end it really depends how many module labels are the same between DDD and DD4Hep setups, and how many are different (as in #34167 (comment)).

@cvuosalo
Copy link
Contributor

@makortel Here's what I am trying in GeometrySimDB_cff.py (and similarly in GeometryRecoDB_cff.py):

from Configuration.ProcessModifiers.dd4hep_cff import dd4hep

def _loadGeometryDBDDD(process) :
    process.load('Configuration.Geometry.GeometryDDDSimDB_cff')

def _loadGeometryDBDD4hep(process) :
    process.load('Configuration.Geometry.GeometryDD4hepSimDB_cff')

modifyGeometryConfiguration = (~dd4hep).makeProcessModifier(_loadGeometryDBDDD)
modifyGeometryConfiguration_dd4hep = dd4hep.makeProcessModifier(_loadGeometryDBDD4hep)

The cff files that are loaded are about 20 lines long. Is that too complex?

@cvuosalo
Copy link
Contributor

Draft PR #34219 submitted for discussion.

@cvuosalo
Copy link
Contributor

I have created a possible config for DD4hep sim from the DB. However, it fails with this exception:

Running EventSetup component DDCompactViewESProducer/'
Exception Message:
No data of type "DDDetector" with label "" in record "IdealGeometryRecord"
 Please add an ESSource or ESProducer to your job which can deliver this data.

It is puzzling, because the configs do contain the needed producers (below). What is missing?

process.DDCompactViewESProducer = cms.ESProducer("DDCompactViewESProducer",
    appendToDataLabel = cms.string('')
)
process.DDDetectorESProducerFromDB = cms.ESSource("DDDetectorESProducer",
    appendToDataLabel = cms.string(''),
    fromDB = cms.bool(True),
    label = cms.string('Extended'),
    rootDDName = cms.string('cms:OCMS')
)

@cvuosalo
Copy link
Contributor

cvuosalo commented Oct 4, 2021

This issue has been resolved. DD4hep workflow 11634.912 reads DD4hep geometry from the DB.

@cvuosalo
Copy link
Contributor

cvuosalo commented Oct 4, 2021

+1

@tvami
Copy link
Contributor

tvami commented Oct 5, 2021

@cms-sw/core-l2 please sign this issue, since it's been resolved. Thanks!

@makortel
Copy link
Contributor

makortel commented Oct 5, 2021

+core

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 5, 2021

This issue is fully signed and ready to be closed.

@qliphy qliphy closed this as completed Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants