-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Segmentation fault with G4 Refitter #31920
Comments
A new Issue was created by @mmusich Marco Musich. @Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks. cms-bot commands are listed here |
assign simulation, reconstruction |
New categories assigned: reconstruction,simulation @mdhildreth,@slava77,@perrotta,@jpata,@civanch you have been requested to review this Pull request/Issue and eventually sign? Thanks |
Additionally, when trying to move the analysis to the master cycle (
I have tried to solve by applying this patch: diff --git a/TrackPropagation/Geant4e/plugins/GeantPropagatorESProducer.cc b/TrackPropagation/Geant4e/plugins/GeantPropagatorESProducer.cc
index 44c960fabdb..c1e0232bf6c 100644
--- a/TrackPropagation/Geant4e/plugins/GeantPropagatorESProducer.cc
+++ b/TrackPropagation/Geant4e/plugins/GeantPropagatorESProducer.cc
@@ -1,6 +1,4 @@
#include "GeantPropagatorESProducer.h"
-#include "MagneticField/Engine/interface/MagneticField.h"
-#include "MagneticField/Records/interface/IdealMagneticFieldRecord.h"
#include "TrackPropagation/Geant4e/interface/Geant4ePropagator.h"
#include "FWCore/Framework/interface/ESHandle.h"
@@ -13,21 +11,17 @@
using namespace edm;
-GeantPropagatorESProducer::GeantPropagatorESProducer(const edm::ParameterSet &p) {
- std::string myname = p.getParameter<std::string>("ComponentName");
+GeantPropagatorESProducer::GeantPropagatorESProducer(const edm::ParameterSet &p):
+ magFieldToken_(setWhatProduced(this, p.getParameter<std::string>("ComponentName")).consumesFrom<MagneticField, IdealMagneticFieldRecord>(edm::ESInputTag("","")))
+{
pset_ = p;
- setWhatProduced(this, myname);
}
GeantPropagatorESProducer::~GeantPropagatorESProducer() {}
std::unique_ptr<Propagator> GeantPropagatorESProducer::produce(const TrackingComponentsRecord &iRecord) {
- ESHandle<MagneticField> magfield;
- iRecord.getRecord<IdealMagneticFieldRecord>().get(magfield);
-
std::string pdir = pset_.getParameter<std::string>("PropagationDirection");
std::string particleName = pset_.getParameter<std::string>("ParticleName");
-
PropagationDirection dir = alongMomentum;
if (pdir == "oppositeToMomentum")
@@ -37,5 +31,5 @@ std::unique_ptr<Propagator> GeantPropagatorESProducer::produce(const TrackingCom
if (pdir == "anyDirection")
dir = anyDirection;
- return std::make_unique<Geant4ePropagator>(&(*magfield), particleName, dir);
+ return std::make_unique<Geant4ePropagator>(&(iRecord.get(magFieldToken_)), particleName, dir);
}
diff --git a/TrackPropagation/Geant4e/plugins/GeantPropagatorESProducer.h b/TrackPropagation/Geant4e/plugins/GeantPropagatorESProducer.h
index 1b41fdd4680..272380e1dd7 100644
--- a/TrackPropagation/Geant4e/plugins/GeantPropagatorESProducer.h
+++ b/TrackPropagation/Geant4e/plugins/GeantPropagatorESProducer.h
@@ -5,6 +5,9 @@
#include "FWCore/ParameterSet/interface/ParameterSet.h"
#include "TrackingTools/GeomPropagators/interface/Propagator.h"
#include "TrackingTools/Records/interface/TrackingComponentsRecord.h"
+#include "MagneticField/Engine/interface/MagneticField.h"
+#include "MagneticField/Records/interface/IdealMagneticFieldRecord.h"
+
#include <memory>
/*
@@ -23,6 +26,7 @@ public:
private:
edm::ParameterSet pset_;
+ edm::ESGetToken<MagneticField, IdealMagneticFieldRecord> magFieldToken_;
};
#endif but I am not sure if these changes are appropriate at all, as running with them, it results in an immediate segmentation fault at the 1st event. |
Looks correct to me by eye.
Could you share the stack trace, or is the same as in the description? |
no, it's different, here is a link to the stack trace.
|
How many threads are you using to run the job? |
process.options = cms.untracked.PSet()
process.options.numberOfThreads = cms.untracked.uint32(1) |
@mmusich , in #32239 and #32240 fixes are proposed. Sorry, that It takes too long but I was extremely busy and debugging takes significant time. How I see the situation now:
In summary: I would propose, to include #32240 in the next new patch release and try to run. When Geant4 10.7 will be out (next week) I will try to work on Geant4e patch making it thread safe and more robust. |
@civanch thanks. |
@civanch I describe it in #32260 (comment) |
@mmusich , can you, please, point to how G4Refitter.py is created. Geometry instantiation was changed some time ago after 10_6 and we likely need to have a slightly different configuration. |
OK, will try to make G4REfitter for 11_2. |
If this file comes from me and if I recall correctly, I derived it from
src/TrackPropagation/Geant4e/python/geantRefit_cff.py, probably in a
10_0_x release.
On 25.11.2020 11:57, Marco Musich wrote:
@civanch <https://github.com/civanch> Rainer Manker ***@***.***
<https://github.com/rmankel>) created it some time ago (I think by
hand). If you tell me how to modify it accordingly I will update it.
Thanks.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#31920 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQFBMXGVK73TZ2Y6A2R2ZTSRTPJTANCNFSM4S4L2YUQ>.
--
---------------------------------------------------------------------
Rainer Mankel | Mail: DESY, CMS, Notkestr 85, D-22603 Hamburg
DESY | Phone: +49 40/8998-2339 Fax: -3092
| Email: [email protected]
---------------------------------------------------------------------
|
@mmusich , when I tried G4Reffiter.py with one of the recent IB and got following problem: %MSG-i HCAL: (NoModuleName) 02-Dec-2020 13:47:41 CET pre-events G4Reffiter itself seems to be relatively simple: import FWCore.ParameterSet.Config as cms process = cms.Process("G4eRefit") process.load('Configuration.StandardSequences.Services_cff') from Configuration.AlCa.GlobalTag import GlobalTag this is necessary to get the simulation geometryprocess.GlobalTag.toGet = cms.VPSet( process.MeasurementTrackerEvent.pixelClusterProducer = 'ALCARECOTkAlMuonIsolated' process.Geant4eTrackRefitter.src = cms.InputTag("ALCARECOTkAlMuonIsolated") The rest of it should not affect this crash. What one should changed in 11_2 in order to run Run-2 data? |
@civanch from Configuration.AlCa.GlobalTag import GlobalTag
process.GlobalTag.globaltag = "112X_dataRun2_v7" by the way I have provided the full configuration updated already once here. DeltaChordTracker = cms.double(0.001), ## in mm
DeltaOneStepTracker = cms.double(1e-4),## in mm
DeltaIntersectionTracker = cms.double(1e-6),## in mm
RmaxTracker = cms.double(8000), ## in mm
ZmaxTracker = cms.double(11000), ## in mm
EnergyThTracker = cms.double(0.2), ## in GeV |
@civanch I was wondering if in the past two weeks there has been any progress on this. |
@civanch, could you please let me know if you have further debugged this problem? |
@mmusich , before the winter break I have started to fix the problem may be not in an optimal way - too general, which require more coherent changes in base classes. I will restart soon with a simple fix. |
@civanch I tested CMSSW_11_3_X_2021-02-08-1100 which contains #32833 and indeed the configuration at https://gist.github.com/mmusich/12227a1a1d90e13ebae9502ac512c7d3 runs fine. |
@mmusich , should I backport the fix to 11_2? |
@civanch indeed, that would be great. |
+1 based on #31920 (comment) |
+1 |
This issue is fully signed and ready to be closed. |
Maybe this issue is fully resolved in more recent CMSSW/geant4 versions, but I've done some more debugging of some rare pathological cases I've still encountered in CMSSW_10_6_X. The problem is that in In g4 cms/v10.4.3 a brute force fix is to resynchronize the voxel node:
|
While the original underlying issue should be fixed by #40543 there are some remaining segfaults when trying to propagate very low pt states (the current minimum momentum protections should avoid this, but should follow up with some lower level protections in the future) |
Dear all,
I'd like to surface here the discussion which has been ongoing for some time in this Hypernews thread.
For reasons linked to avoiding biases in the alignment of the Tracker End Caps (apparently induced by bad description of the Tracker RECO material in the Barrel / Encap regions), we'd like to run the Geant4e track refitter in order to be able to use the more accurate description of the material from the simulation geometry.
This approach works fine with MC, but unfortunately when trying it on real data several issues have been found.
A first issue, in connection with
sim::Field::GetFieldValue
was solved by this PR #31203.After applying the fix, several other segmentation faults have appeared, all generally related to the Geant4e refitter.
More information can be found in this talk.
A minimal reproducer (which does only the G4 refit) is available here.
Testing it in
CMSSW_10_6_X_2020-10-22-1100
a segfault occurs after ~45 minutes of processing around:186101st record. Run 316153, Event 817473248, LumiSection 606
.Trying to skip with the
PoolSource
to that event in particular, the segmentation fault does not appear anymore, seemingly indicating the issue is linked to the previous processing history.The stack trace proceeds as follows at this link with the relevant part being:
Attention from Simulation (and possibly Reconstruction) group-s might be needed.
Any help with this issue is highly appreciated.
cc:
@rmankel @vbotta @connorpa
The text was updated successfully, but these errors were encountered: