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

Prepare for taking MF configurations from DB #6514

Closed
wants to merge 3 commits into from

Conversation

namapane
Copy link
Contributor

Includes:
-Tools to create .db files for standard configurations
-Fix for one numerical stability problem with geometries imported to DB. The problem did not affect existing configurations (with geometry coming from xml), and the fix was regression-tested.

This fix is needed to be able to finally switch to reading MF geometry and configuration from DB, which will happen in a future (pre)release once a GT with all required payloads becomes available.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @namapane (Nicola Amapane) for CMSSW_7_3_X.

Prepare for taking MF configurations from DB

It involves the following packages:

CondFormats/MFObjects
MagneticField/Engine
MagneticField/GeomBuilder
MagneticField/Layers

@nclopezo, @cerminar, @cmsbuild, @diguida, @rcastello, @StoyanStoynev, @slava77, @mmusich can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cmsbuild
Copy link
Contributor

-1
Tested at: 8efe88d
When I ran the RelVals I found an error in the following worklfows:
25202.0 step5

runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15+MINIAODMC/step5_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15+MINIAODMC.log
----- Begin Fatal Exception 21-Nov-2014 10:18:07 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=NjettinessAdder label='Njettiness'
Exception Message:
MissingParameter: Parameter 'measureDefinition' not found.
----- End Fatal Exception -------------------------------------------------

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6514/1033/summary.html

@namapane
Copy link
Contributor Author

This PR is not related to anything relayed to "NjettinessAdder" nor it
has anything to do with any "measureDefinition" whatsoever. What's this
crap?

Nicola

On 21-Nov-14 10:18, cmsbuild wrote:

-1
Tested at: 8efe88d
8efe88d
When I ran the RelVals I found an error in the following worklfows:
25202.0 step5

runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15+MINIAODMC/step5_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15+MINIAODMC.log

----- Begin Fatal Exception 21-Nov-2014 10:18:07 CET-----------------------

An exception of category 'Configuration' occurred while

[0] Constructing the EventProcessor

[1] Constructing module: class=NjettinessAdder label='Njettiness'

Exception Message:

MissingParameter: Parameter 'measureDefinition' not found.

----- End Fatal Exception -------------------------------------------------

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6514/1033/summary.html


Reply to this email directly or view it on GitHub
#6514 (comment).

@namapane
Copy link
Contributor Author

So this is a feature introduced by @davidlange6 in an unrelated commit:

c8cc267#diff-248d253292a18c6b288e1ce71fe60d82

it seems it breaks the tests, because as far as I can tell it requires a change in config parameters for runTheMatrix.

Can you please fix this so that we can move on?

@davidlange6
Copy link
Contributor

think of my commit as partly fixing a problem rather than completely fixing it. The complete fix is now in the IB being built and the failure won't hold up this PR

On Nov 21, 2014, at 12:32 PM, Nicola Amapane notifications@github.com
wrote:

So this is a feature introduced by @davidlange6 in an unrelated commit:

c8cc267#diff-248d253292a18c6b288e1ce71fe60d82

it seems it breaks the tests, because as far as I can tell it requires a change in config parameters for runTheMatrix.

Can you please fix this so that we can move on?


Reply to this email directly or view it on GitHub.

#ifdef MF_DEBUG
{
MagVolume6Faces* mv = static_cast<MagVolume6Faces*> (*ivol);
cout << " Trying volume " << mv->volumeNo << " " << int(mv->copyno) << endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

cout is not recommended (including due to thread-safety considerations), there are the various LogMessages available. I suppose at some point there will be a campaign to clean the code but we can at least avoid introducing more to clean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is in a #ifdef, and needs to stay like this, because this debug mode activates additional checks that need not to be run in production.

@diguida
Copy link
Contributor

diguida commented Nov 24, 2014

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_3_X IBs unless changes (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @nclopezo, @ktf, @smuzaffar

@ktf
Copy link
Contributor

ktf commented Nov 26, 2014

This has now been merged into CMSSW_7_4_X by #6602. Please close this if you do not plan to backport it to CMSSW_7_3_X.

@namapane
Copy link
Contributor Author

OK, assuming this won't be needed in 73X at this point, I close it. Thanks for taking care of 74X.

@namapane namapane closed this Nov 26, 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