search

cms-sw / cmssw

CMS Offline Software
http://cms-sw.github.io/
Apache License 2.0
1.07k stars 4.28k forks source link

re-enable LogWarnings in MkFitOutputConverter #35803

Open slava77 opened 2 years ago

slava77 commented 2 years ago

this is to follow up after #35802 (temporarily) silenced the warnings. The actions on this issue can be taken after a new release of mkFit includes some reasonable cleanup of the output tracks

@mmasciov @osschar @leonardogiannini @makortel @cms-sw/tracking-pog-l2

slava77 commented 2 years ago

assign reconstruction

cmsbuild commented 2 years ago

New categories assigned: reconstruction

@slava77,@jpata you have been requested to review this Pull request/Issue and eventually sign? Thanks

cmsbuild commented 2 years ago

A new Issue was created by @slava77 Slava Krutelyov.

@Dr15Jones, @perrotta, @dpiparo, @makortel, @smuzaffar, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

mmasciov commented 2 years ago

While PR #35974 is partially addressing this issue, a non-negligible amount of mkFit-related warnings is found when reverting https://github.com/cms-sw/cmssw/pull/35802/files#diff-23488a14c37419124c78f0e1d710cf10851f20228de09766027ccd47f9563414R301. As more investigation is required, for the moment PR #35802 is not reverted.

mmusich commented 2 years ago

Now that https://github.com/cms-sw/cmssw/issues/35798 is closed thanks to https://github.com/cms-sw/cmssw/pull/37139, shall this issue be taken into account?

slava77 commented 2 years ago

Now that #35798 is closed thanks to #37139, shall this issue be taken into account?

I'm not sure anymore, perhaps this issue needs to be reformulated.

The implications for just reenabling the warnings in MkFitOutputConverter would be to move the selections and checks from MkFitOutputConverter to MkFitProducer. Related to the covariances, I'm not quite sure that checks of positive definiteness will work well on that side because the state covariance is in single precision in the mkFit representation and in a 6x6 representation the same determinant check will not work. So, some checks would have to stay in MkFitOutputConverter anyways (without a LogWarning).

There is a plan to add similar sanity checks on the state parameters inside of the mkFit track pattern recognition. This will come a bit later and this issue can be a checkpoint for that work.

jpata commented 2 years ago

type tracking

mmusich commented 2 years ago

I think the type should be tracking and not trk here.