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

81x: add pixel barycenter computation tool #14730

Merged

Conversation

mmusich
Copy link
Contributor

@mmusich mmusich commented Jun 1, 2016

Making available in release tool needed for Pixel barycenter computation:
https://twiki.cern.ch/twiki/bin/view/CMS/TkAlignmentPixelPosition

attn: @tlampen, @mschrode,
@ghellwig please comment if there is any other more suitable place to put this.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2016

A new Pull Request was created by @mmusich (Marco Musich) for CMSSW_8_1_X.

It involves the following packages:

Alignment/MillePedeAlignmentAlgorithm

@ghellwig, @cerminar, @cmsbuild, @franzoni, @mmusich, @davidlange6 can you please review it and eventually sign? Thanks.
@mschrode, @ghellwig, @tocheng, @tlampen this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@mmusich mmusich changed the title 80x: add pixel barycenter computation tool 81x: add pixel barycenter computation tool Jun 1, 2016
@mmusich
Copy link
Contributor Author

mmusich commented Jun 1, 2016

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/13307/console

@mschrode
Copy link

mschrode commented Jun 1, 2016

@mmusich @ghellwig @tlampen How about moving it to Alignment/TrackerAlignment/macros?

@ghellwig
Copy link

ghellwig commented Jun 1, 2016

@mmusich do I understand it correctly that you rely on MillePede only with this tool? If yes, I think it is already at the right place. If not, @mschrode's suggestion looks reasonable.

@mmusich
Copy link
Contributor Author

mmusich commented Jun 1, 2016

@ghellwig yes, basically was an add-on from Gero Flucke to quickly derive the barycenter position, designed to be included in MillePedeAlignmentAlgorithm making use of the existing suite of macros availble in Alignment/MillePedeAlignmentAlgorithm/macros/GFUtils/.
There is no particular reason in general to use MillePede to do this job, but since the parsing of the output was already at hand...
I've just updated the configuration to make it work in current releases.

@ghellwig
Copy link

ghellwig commented Jun 1, 2016

@mmusich ok, this means, in its current implementation it works only with MillePede, but in principal it could be generalized to use, let's say, the resulting sqlite file instead of the millepede.res file, right?
I would then say that the current location is fine.

@ghellwig
Copy link

ghellwig commented Jun 1, 2016

Before approving, I would like to hear the final opinion of @mschrode and @tlampen

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2016

@tlampen
Copy link
Contributor

tlampen commented Jun 1, 2016

For me it seems that the directory MillePedeAlignmentAlgorithm/macros/ contains only files related to MPS. Therefore TrackerAlignment/macros would sound a more consistent location. Does the depencency of MillePede cause difficulties in that location?

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2016

Pull request #14730 was updated. @ghellwig, @cerminar, @cmsbuild, @franzoni, @mmusich, @davidlange6 can you please check and sign again.

@mmusich
Copy link
Contributor Author

mmusich commented Jun 1, 2016

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/13322/console

@mmusich
Copy link
Contributor Author

mmusich commented Jun 1, 2016

@ghellwig @mschrode @tlampen I moved the configuration to Alignment/TrackerAlignment/test. The macro stays in MillePedeAlignmentAlgorithm where I think it should belong in any case. The Alignment/TrackerAlignment/macros sub-package in any case doesn't exist.

@mschrode
Copy link

mschrode commented Jun 1, 2016

@mmusich @ghellwig @tlampen OK I was not aware the script depends on tools in MillePedeAlignmentAlgorithm/macros. Then I fully agree, it should stay there.

@ghellwig
Copy link

ghellwig commented Jun 1, 2016

+1
provided jenkins agrees

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2016

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2016

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 10344c1 into cms-sw:CMSSW_8_1_X Jun 2, 2016
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

6 participants