add ignoreVertices parameter #18041
add ignoreVertices parameter #18041
Conversation
…ng use of the PV collection
|
A new Pull Request was created by @mtosi (mia tosi) for master. It involves the following packages: RecoTracker/FinalTrackSelectors @perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here #13028 |
|
please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
| @@ -41,18 +43,19 @@ TrackMVAClassifierBase::TrackMVAClassifierBase( const edm::ParameterSet & cfg ) | |||
|
|
|||
| } | |||
|
|
|||
|
|
|||
| #include "DataFormats/VertexReco/interface/Vertex.h" | |||
There was a problem hiding this comment.
Is there any reason for having this #include here, and not on top together with the other ones?
There was a problem hiding this comment.
shouldn't compile faster in this way ?
but I might be wrong
There was a problem hiding this comment.
#include is a preprocessing directive, and therefore compilation time shouldn't depend on where it is placed.
I think it can be a good practice to put all them together at the beginning of the cpp file, so that they are all listed together and not scattered around the file, which makes easier checking whether something is missing or placed at a wrong order. At this point it is mostly a matter of style, I admit, but it can help when the complexity of the code increases.
cmsbuild
added
comparison-pending
tests-pending
and removed
comparison-available
tests-approved
labels
|
Pull request #18041 was updated. @perrotta, @cmsbuild, @slava77, @davidlange6 can you please check and sign again. |
|
done, hope it is fine, now |
|
please test |
|
The tests are being triggered in jenkins. |
cmsbuild
added
comparison-pending
tests-pending
and removed
comparison-notrun
tests-rejected
labels
|
Pull request #18041 was updated. @perrotta, @cmsbuild, @slava77, @davidlange6 can you please check and sign again. |
|
please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
+1 |
cmsbuild
added
fully-signed
reconstruction-approved
and removed
pending-signatures
reconstruction-pending
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 requires discussion in the ORP meeting before it's merged. @Muzaffar, @davidlange6, @smuzaffar |
|
@perrotta I checked 25202.0, and already TrackingParticles (=simulation) shows differences |
|
step2 in baseline and this PR in 25202 apparently have different minbias inputs there are no xrootd errors related to it. So, my guess is there may be random number changes in digis. These would be more likely due to bad or uninitialized memory problems, which could be clarified with valgrind. |
| computeMVA(tracks,*hBsp,*hVtx,forest,*mvas); | ||
| } else { | ||
| if ( !ignoreVertices_ ) | ||
| edm::LogWarning("TrackMVAClassifierBase") << "ignoreVertices is set to False in the configuration, but the vertex collection is not valid"; |
There was a problem hiding this comment.
hi @mtosi is the log warning meant to indicate a "ok" configuration or a job misconfiguration?
There was a problem hiding this comment.
ciao
it should indicate a misconfiguration, indeed
the module is configured to make use of the vertices collection,
while the corresponding InputTag is not valid
is there anything wrong w/ this approach ?
do you have any suggestions on how to improve it ?
thanks !
|
+1 |
for handling the possibility of not making use of the PV collection
as requested by MUO POG