HBHE: M2 small fixed #21821
HBHE: M2 small fixed #21821
Conversation
|
The code-checks are being triggered in jenkins. |
mariadalfonso
force-pushed
the
PRcleanupM2
branch
from
33641e5
to
edfedd5
Compare
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-21821/2819 |
|
A new Pull Request was created by @mariadalfonso for master. It involves the following packages: HLTrigger/Configuration @perrotta, @cmsbuild, @silviodonato, @slava77, @Martin-Grunewald, @fwyzard can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-21821/2820 |
|
On 1/9/18 3:35 PM, mariadalfonso wrote:
# use mahi as default only in era 2018 and following, while for the
previous eras 2015,2016,2017 prefer to keep M2 to avoid to recalibration
please reapply to all.
The strategy for all reco algorithmic updates is to apply the same
algorithm to all data and MC in the same release unless the detector
changes make this choice impossible.
|
|
@Martin-Grunewald |
|
You can use python's |
mariadalfonso
force-pushed
the
PRcleanupM2
branch
from
edfedd5
to
6255173
Compare
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-21821/2824 |
|
Pull request #21821 was updated. @perrotta, @cmsbuild, @silviodonato, @slava77, @Martin-Grunewald, @fwyzard can you please check and sign again. |
|
@slava77 @Martin-Grunewald |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
@slava77: this part of the code was not included in the original PR.
Since some dead code got reported by the static analyzer, I asked
Maria to fix it. The whole class indeed contains a lot of commented
out couts, besides the ones exposed here, that should be better fixed
as you are suggesting. Let do it in a forthcoming PR, once this one is
merged (hopefully as soon as the 10_1_X queue is opened, as I don't
see any reason to delay it further). In the meanwhile let it as such,
with the adjustments that are integrated here.
Slava Krutelyov <notifications@github.com> ha scritto:
… slava77 commented on this pull request.
> @@ -324,8 +324,8 @@ bool HybridMinimizer::Minimize() {
// print the real number of maxfcn used (defined in
ModularFuncitonMinimizer)
int maxfcn_used = maxfcn;
if (maxfcn_used == 0) {
- int nvar = fState.VariableParameters();
- maxfcn_used = 200 + 100*nvar + 5*nvar*nvar;
+ // int nvar = fState.VariableParameters();
if the code is needed for debugging, just add an appropriate switch
to enable/disable it (it can be a configuration flag or a compiler
macro). With a config switch you get some assurance of future
maintenance of that code.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#21821 (comment)
|
|
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
|
+1
|
|
+1 |
cmsbuild
added
fully-signed
hlt-approved
and removed
hlt-pending
pending-signatures
labels
|
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) |
|
@mariadalfonso @deguio @slava77 @perrotta I think we are ready for integration in 10_1_X. Do we need it also in 10_0_X? I understand from the thread and private discussions that the net effect should be minor. |
|
+1 |
|
@fabiocos : I'd let HCAL and TSG (@Martin-Grunewald @silviodonato ,
since the tiny changes only apply to M2) to decide.
I think the idea was not to backport; but I personally see no
counterindications in having in 10_0_X the whole set of HCAL PRs that
were submitted for the initial stage of the development for 2018.
Fabio Cossutti <notifications@github.com> ha scritto:
… @mariadalfonso @deguio @slava77 @perrotta I think we are ready for
integration in 10_1_X. Do we need it also in 10_0_X? I understand
from the thread and private discussions that the net effect should
be minor.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#21821 (comment)
|
|
@perrotta this change is clearly not available in the ongoing 10_0_0 validation but will be present in production if we back-port |
|
Fabio Cossutti <notifications@github.com> ha scritto:
@perrotta this change is clearly not available in the ongoing 10_0_0
validation but will be present in production if we back-port
That's why I think TSG should have a word on it!
The easiest thing to do is obviously not to backport: therefore, if
nobody requests it we shouldn't
… --
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#21821 (comment)
|
in this PR