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

From cmssw 7 1 x phase1 upgrade #2578

Merged
merged 40 commits into from Mar 5, 2014
Merged

From cmssw 7 1 x phase1 upgrade #2578

merged 40 commits into from Mar 5, 2014

Conversation

iasonastopsis
Copy link
Contributor

Code for the phase1 Upgrade

@davidlange6
Copy link
Contributor

I think its best to leave the code in the SLHC release until this new detId scheme happens (not sure why that changes things)

On Feb 21, 2014, at 12:47 PM, Iasonas Topsis notifications@github.com
wrote:

Hello, as was agreed, the first version of the code will have the flag and at a later point it is going to be replaced, once we have the new detId scheme.


Reply to this email directly or view it on GitHub.

@iasonastopsis
Copy link
Contributor Author

So this means you do not want this code in the 7 release, but only in SLHC release?

@danduggan
Copy link
Contributor

What is the timescale of the new detId scheme implementation? At this point I think we would risk having developments in these packages that would not go into the SLHC release, causing much more work down the road than changing these flags.

@davidlange6
Copy link
Contributor

these developments have been in the slhc release for some time no? (I believe I made the same comment at the time they went into that release).

but - can you answer what part of the det id work this is tied to and why it can't be already derived from the geometry? [even making a pset with that can be loaded as a customize function with the needed geometry information would be a much cleaner solution]

On Feb 21, 2014, at 12:58 PM, Iasonas Topsis notifications@github.com
wrote:

So this means you do not want this code in the 7 release, but only in SLHC release?


Reply to this email directly or view it on GitHub.

@danduggan
Copy link
Contributor

Can you point the developers to a clear example of code that uses the geometry as a flag? If this is a quick implementation of this example, then the developers could put this in and resubmit the request. Otherwise, I'd much prefer to accept this request now and have them update the code in a week or two with this update. The changes needed for the SLHC release touch quite a few of these pixel packages, so having this in now is much better for other pixel code requests that will be coming.

@iasonastopsis
Copy link
Contributor Author

Hello, the general structure of the upgrade code is the following:
-- We have 2 new classes in DataFormats/SiPixelDetId, i.e. PixelBarrelNameUpgrade.cc and PixelEndcapNameUpgrade.cc which contain the matching for the upgrade.
-- In each ...source.cc file in the DQM packages we take the isUpgrade flag in the constructor.
-- Where the histograms are booked/filled there is an if(isUpgrade) statement, so if the flag is true the new structure is constructed.

@davidlange6
Copy link
Contributor

On Feb 21, 2014, at 1:27 PM, danduggan notifications@github.com
wrote:

Can you point the developers to a clear example of code that uses the geometry as a flag? If this is a quick implementation of this example, then the developers could put this in and resubmit the request. Otherwise, I'd much prefer to accept this request now and have them update the code in a week or two with this update. The changes needed for the SLHC release touch quite a few of these pixel packages, so having this in now is much better for other pixel code requests that will be coming.

I fear I don't know what specific characteristics are needed - but here is an example to get started

const TrackerGeometry* theTracker;
for ( trkIterator = theTracker->detUnits().begin();
trkIterator != theTracker->detUnits().end();
++trkIterator ) {

DetId id = (**trkIterator).geographicalId();

// then query the det id for all the attributes you would like so that you can find what the maximum layer number in the pxb/pxf/toc/tec/tib/tid is etc

}

as far as I see, this information is independent of the DetId scheme itself and can be retrieved without too much pain at the start of a job (perhaps best put into a producer if needed by a number of modules).


Reply to this email directly or view it on GitHub.

@davidlange6
Copy link
Contributor

I know:) its what we need to change such that the same code can be used for everything (I believe I communicated this quite some time ago)

On Feb 21, 2014, at 2:05 PM, Iasonas Topsis notifications@github.com
wrote:

Hello, the general structure of the upgrade code is the following:
-- We have 2 new classes in DataFormats/SiPixelDetId, i.e. PixelBarrelNameUpgrade.cc and PixelEndcapNameUpgrade.cc which contain the matching for the upgrade.
-- In each ...source.cc file in the DQM packages we take the isUpgrade flag in the constructor.
-- Where the histograms are booked/filled there is an if(isUpgrade) statement, so if the flag is true the new structure is constructed.


Reply to this email directly or view it on GitHub.

@cmsbuild
Copy link
Contributor

@nclopezo nclopezo modified the milestones: CMSSW_7_1_0_pre4, CMSSW_7_1_0_pre3 Feb 24, 2014
@danduggan
Copy link
Contributor

+1

@civanch
Copy link
Contributor

civanch commented Mar 5, 2014

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2014

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

ktf added a commit that referenced this pull request Mar 5, 2014
@ktf ktf merged commit debd407 into cms-sw:CMSSW_7_1_X Mar 5, 2014
@nclopezo nclopezo modified the milestones: CMSSW_7_1_0_pre5, CMSSW_7_1_0_pre4 Mar 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants