-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Crash in DD4hep geometry, step2 #36837
Comments
A new Issue was created by @fabiocos Fabio Cossutti. @Dr15Jones, @perrotta, @dpiparo, @makortel, @smuzaffar, @qliphy can you please review it and eventually sign/assign? Thanks. cms-bot commands are listed here |
assign geometry |
New categories assigned: geometry @cvuosalo,@mdhildreth,@ianna,@Dr15Jones,@makortel,@civanch you have been requested to review this Pull request/Issue and eventually sign? Thanks |
assign upgrade |
New categories assigned: upgrade @AdrianoDee,@srimanob you have been requested to review this Pull request/Issue and eventually sign? Thanks |
This issue seems a bit strange and I didn't see it before as I reported since last Oct. So now, I have additional clues on step-2 (DIGI):
One can try with (*) (**) |
Hi,
Both out-of-the-box configs come with a call to XMLIdealGeometryESSource in TrackTriggerSetup. The config is at https://github.com/cms-sw/cmssw/blob/master/L1Trigger/TrackerDTC/python/ProducerES_cfi.py#L13-L17
But only in DDD config that has the following ESSource in the config,
For DD4Hep case, there is
I think for the DD4hep case, there should be a modifier somewhere that this ideal geometry source will be added to the config. With the manual update ( FYI @cvuosalo @cms-sw/l1-l2 @bsunanda (*)
|
By the way, this is a stack trace before the crash in
FYI @cms-sw/dt-dpg-l2 |
Hi @perrotta @qliphy @civanch @makortel @cvuosalo @bsunanda I've listed issues that I found in DD4hep phase-2 workflow in How do you want me to deal with the report on git issue? Should we move one-by-one in separate issues or else? Since some crashes appear after my manual fixes of other issues, it will be a bit complicated to explain in ticket-by-ticket. Thanks for your advices. |
@srimanob thank you for your investigations |
Following the discussion at last simulation meeting, I checked scenario D88 for MTD only (see https://github.com/fabiocos/cmssw/tree/fc-testmtdD88 ) and I confirm there is a crash in the reconstruction geometry of MTD:
To be understood and fixed |
I've pinged @cms-sw/dt-dpg-l2 @cms-sw/l1-l2 on the issue also on DIGI-L1_HLT step. |
I believe I have understood the cause of the crash in the dd4hep version of MTD D88, but I am not sure about the way out. In D88 ETL unfortunately needs to use boolean solids, starting with the main mother volume When a boolean solid is found, this uses an obscure (at least for me) logic within our DD4hep interface, see https://github.com/cms-sw/cmssw/blob/master/DetectorDescription/DDCMS/src/DDFilteredView.cc#L544 In the ETL case there is a boolean union o a boolean union of a polycone and of a trapezoid. Due to the implemented logic there is an iterative search for boolean composites unless the left shape is a box, which is not the case for ETL. And of course when the left shape is a polycone and the This logic was implemented by @ianna some 3 years ago, and I am not sure to which extent it was tested in practice. In any case, there seems to be an implicit assumption that any boolean composition must collapse into the parameters of a box, unless I misinterpret it, why? |
This modification to the
I cannot say whether it makes any sense for other use cases, in ETL the boolean solids involve just the main envelope and passive materials (of course the printouts are there just for debugging purposes). |
@civanch well, the above code is in |
@fabiocos - there used to be only one case of the boolean shapes needed that logic. I think, one can add the ETL-specific case to retrieve the parameters, or generalise it there. The latter will be less performant. This is if we do not want to use the DD4hep native parameters (that came later). |
@ianna , do I understand you correctly that DD4hep has limitations for union solids? If yes, is it a fundamental problem or simply not addressed one? |
@fabiocos Is this problem an opportunity to remove the error-prone sequence of anonymous numbers called |
@ianna are you referring to the internal structure of In practice at present what would do the job and unblock scenario D88 is to add a specific protection in the code above accounting for both the case you designed and this one. |
sorry, I see that the question was by @cvuosalo ... |
Test with @fabiocos, the step-3 can pass MTD reco geometry construction now. We move to the next issue. Gdoc is updaetd. |
The fix on GEM RECO geometry construction comes in #36941 |
The fix for MTD in #36970 looks good. Thanks @fabiocos. I'll create an issue for the possible re-design of |
Following up on the DT issue (from email discussion with @cms-sw/dt-dpg-l2), we first focus on simMuonDTDigis. The DT detIDs have not been built properly in Phase2 DD4hep. We got a total of 2780 (Chamber+SL+Layer) instead of 3650. Looking at
in https://github.com/cms-sw/cmssw/blob/master/Geometry/DTGeometryBuilder/src/DTGeometryBuilderFromDD4hep.cc#L60-L65 do not return as expected. For example, Consider that
we need to debug more since the issue seems to appear only on Phase-2 DD4hep. To reproduce the issue one can use the following cmsDriver with CMSSW_12_3_0_pre5, Digi-only DDD: |
Hi @srimanob can you please write the cmsDriver.py options to reproduce your step1-DD4hep.root ? I'd like to reproduce the issue starting from a GEN-SIM .root file. |
I pick it from the wf 39434.911, i.e.
|
My test PR on DD4hep for L1Track step #37005 |
Hi All, I've proposed an update on
Now, we have error/warning message from HCAL to look at. FYI @cms-sw/hcal-dpg-l2 Message in DIGI step:
Message in RECO step:
The summary on how to reproduce the result is in (*) (*)
|
@srimanob the issue in itself looks closed now that the workflow can run through all steps without crashes. Residual warning/error messages seem to go beyond the scope of this issue, if meaningful results can be generally produced. Therefore once all the fixes are integrated, I would prefer to close this issue, and move possible further discussions about residual problems to some dedicated thread/issue. |
I've opened the issue #37087 to collect issues/problems in DDD-DD4hep Phase-2 validation. |
@srimanob since all the PR listed above have been merged, I consider this issue as solved. |
+1 |
As reported by @bsunanda at the latest SIM meeting, DD4hep geometries succeed in step1, but fail in phase2 step2, see geometry D77 wf 35034.911
The text was updated successfully, but these errors were encountered: