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.

Sign up for GitHub

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

Fixing initialization of event setup point of PR 6030 #6085

Merged
merged 2 commits into from
Nov 5, 2014
Merged

Fixing initialization of event setup point of PR 6030 #6085

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9e1929
Fixing overdoing of EventSetup pointer initialization
mmusich Oct 29, 2014
1b280ff
reverting commits b9e1929c5e8b0269eef89c0c4feaa1bbb8712f58 and 078138…
mmusich Nov 4, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Alignment/OfflineValidation/plugins/TrackerOfflineValidation.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,6 @@ TrackerOfflineValidation::~TrackerOfflineValidation()
void
TrackerOfflineValidation::checkBookHists(const edm::EventSetup& es)
{
lastSetup_ = &es;
es.get<TrackerDigiGeometryRecord>().get( tkGeom_ );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The framework does not guarantee that we will always keep the same EventSetup object throughout the job. Please close this commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure to understand the comment. We just need the EventSetup pointer once in the endjob to invoke the TrackerTopology. Before this commit tlampen@078138b, lastSetup_ was simply a nullptr, which was causing seg faults.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't use the EventSetup in the endJob, nor can you use any data held by the EventSetup. That is why it is not passed as a value to the various module's endJob. Please find another way to accomplish what you are trying to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification. Does this imply we should revert also commit 078138b from @tlampen in PR #6030 (which, by the way has already been merged) and in all the backport PR #6070 #6072 and #6073?

const TrackerGeometry *newBareTkGeomPtr = &(*tkGeom_);
if (newBareTkGeomPtr == bareTkGeomPtr_) return; // already booked hists, nothing changed
Expand Down