Ecal shape db #23600
Ecal shape db #23600
Conversation
|
The code-checks are being triggered in jenkins. |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23600/5208 Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23600/5208/git-diff.patch You can run |
|
there are 4K lines of text files with generated numbers. |
|
@slava77 if you are referring to those files: CondTools/Ecal/python/APD_SimPulseShape.txt those are used to create the sqlite3 file, which is later-on passable to create the DB conditions. For me, they are not essential to stay in the release and can be removed, if needed. But for future reference and to allow other developers to see how the code works, it might be good to have them. However, the code works without them. |
|
I see that I need to run the and update the PR. Should I do that already now, or wait for all the release managers to comment in present version of the PR and submit the second PR only afterwards hopefully integrating all comments received ? |
|
On 6/15/18 11:01 AM, Konstantinos Theofilatos wrote:
I see that I need to run the
|-code-checks Logs:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23600/5208 Code check
has found code style and quality issues which could be resolved by
applying a patch in
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23600/5208/git-diff.patch
e.g. curl
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23600/5208/git-diff.patch
| patch -p1 |
and update the PR. Should I do that already now, or wait for all the
release managers to comment in present version of the PR and submit the
second PR hopefully integrating all comments received ?
your PR is actually "invisible" to reviewers until code checks pass
(unless someone watches the full repository)
|
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23600/5225 |
|
A new Pull Request was created by @theofil (Konstantinos Theofilatos) for master. It involves the following packages: CalibCalorimetry/EcalSRTools @perrotta, @cmsbuild, @civanch, @vazzolini, @kmaeshima, @arunhep, @mdhildreth, @dmitrijus, @nsmith-, @rekovic, @franzoni, @jfernan2, @cerminar, @thomreis, @slava77, @ggovi, @pohsun, @vanbesien, @lpernie can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
+1 |
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
please test |
cmsbuild
added
comparison-pending
tests-pending
and removed
comparison-available
tests-approved
labels
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
+1 |
This is about an improved ECAL Shape Class to be used upon digitization. It supports sub nanosecond precision in the ECAL signal pulses and is fully configurable from the DB, in view of the Phase II upgrade where the detector will have a shorter pulse shape. More details can be found in my ECAL DPG presentation:
https://indico.cern.ch/event/731948/
The new shape class supports also the old style Phase-I logic that makes no attempt to fetch information from the DB. In this pull request the code is committed with the DB usage currently de-activated, to allow easier approval of the PR. A second PR is foreseen once this PR is approved (and thus also the new DB Ecal Object can go in the global tag), to actually enable the DB configuration and read the conditions from the corresponding global tag.
I have validated the resulting output with a small scale ZEE MC production as well as a dedicated tool that instantiates ad-hoc pulses.