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

Update the dataformat for the TP per crystal. It now follows what pre… #17669

Merged
merged 3 commits into from Mar 20, 2017

Conversation

nancymarinelli
Copy link
Contributor

…sented to the L1-phase II workshop Dec 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 1, 2017

A new Pull Request was created by @nancymarinelli for CMSSW_9_0_X.

It involves the following packages:

DataFormats/EcalDigi
SimCalorimetry/EcalEBTrigPrimAlgos
SimCalorimetry/EcalEBTrigPrimProducers

@civanch, @mdhildreth, @cmsbuild, @rekovic, @kpedro88, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@argiro this is something you requested to watch as well.
@davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@kpedro88
Copy link
Contributor

kpedro88 commented Mar 1, 2017

@nancymarinelli a few comments:

  1. please remove all commented-out code (from both C++ and Python files)
  2. ROOT files should not be tracked in Git unless absolutely necessary - if the two ROOT files added in this PR are not necessary, the commit needs to be remade without adding these files so they never appear in the history (otherwise they inflate the size of the repository forever)

@kpedro88
Copy link
Contributor

kpedro88 commented Mar 1, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 1, 2017

The tests are being triggered in jenkins.

@nancymarinelli
Copy link
Contributor Author

nancymarinelli commented Mar 1, 2017 via email

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 1, 2017

Pull request #17669 was updated. @civanch, @mdhildreth, @cmsbuild, @rekovic, @kpedro88, @mulhearn, @davidlange6 can you please check and sign again.

@kpedro88
Copy link
Contributor

kpedro88 commented Mar 1, 2017

@nancymarinelli as I stated in my previous comment, doing git rm *.root in a subsequent commit is not sufficient. The files will still take up space in the history as long as they are in any commit in the PR. The initial commit needs to be remade somehow (by hand, or using git tools like interactive rebase, amend, or squashing options) so the files are never tracked.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 1, 2017

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 1, 2017

Comparison job queued.

@nancymarinelli
Copy link
Contributor Author

nancymarinelli commented Mar 1, 2017 via email

@kpedro88
Copy link
Contributor

kpedro88 commented Mar 1, 2017

There are several ways to accomplish this using git. Here is one example based on "squashing" (i.e. combining) the two existing commits:

git checkout -b TP_PhaseII_V1_backup
git checkout TP_PhaseII_V1
git reset --soft HEAD~2
git status -s
git commit -m "Update the dataformat for the TP per crystal. It now follows what presented to the L1-phase II workshop Dec 2016"
git push -f my-cmssw TP_PhaseII_V1

Here you make a backup branch (in case of problems), use reset --soft to undo the last two commits while keeping all the changes, and then recombine the changes into one PR. Since the history is changed, you have to use push -f when sending to the remote. This works for our current purpose because the git add and git rm cancel out when combined in one commit.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 1, 2017

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2017

Pull request #17669 was updated. @civanch, @mdhildreth, @cmsbuild, @rekovic, @kpedro88, @mulhearn, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 6, 2017

Pull request #17669 was updated. @civanch, @mdhildreth, @cmsbuild, @rekovic, @kpedro88, @mulhearn, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 17, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18513/console Started: 2017/03/17 11:51

@cmsbuild
Copy link
Contributor

-1

Tested at: 3d0c710

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

I found follow errors while testing this PR

Failed tests: Build

  • Build:

I found an error when building:

>> Building edm plugin tmp/slc6_amd64_gcc530/src/Validation/EcalRecHits/src/ValidationEcalRecHits/libValidationEcalRecHits.so
Copying tmp/slc6_amd64_gcc530/src/SimCalorimetry/EcalTestBeamAlgos/src/SimCalorimetryEcalTestBeamAlgos/libSimCalorimetryEcalTestBeamAlgos.so to productstore area:
@@@@ Running edmWriteConfigs for ValidationPerformance
In file included from /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2017-03-15-1100/src/DQM/L1TMonitor/interface/L1TOMDSHelper.h:6:0,
                 from /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2017-03-15-1100/src/DQM/L1TMonitor/src/L1TOMDSHelper.cc:1:
/cvmfs/cms-ib.cern.ch/nweek-02463/slc6_amd64_gcc530/cms/cmssw-patch/CMSSW_9_0_X_2017-03-15-1100/src/CondTools/L1Trigger/interface/OMDSReader.h:57:23: error: 'AttributeList' is not a member of 'coral'
    const std::vector< coral::AttributeList >& attLists )
                       ^
/cvmfs/cms-ib.cern.ch/nweek-02463/slc6_amd64_gcc530/cms/cmssw-patch/CMSSW_9_0_X_2017-03-15-1100/src/CondTools/L1Trigger/interface/OMDSReader.h:57:23: error: 'AttributeList' is not a member of 'coral'
/cvmfs/cms-ib.cern.ch/nweek-02463/slc6_amd64_gcc530/cms/cmssw-patch/CMSSW_9_0_X_2017-03-15-1100/src/CondTools/L1Trigger/interface/OMDSReader.h:57:44: error: template argument 1 is invalid
    const std::vector< coral::AttributeList >& attLists )


@cmsbuild
Copy link
Contributor

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

@kpedro88
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 17, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18519/console Started: 2017/03/17 17:01

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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