search

cms-L1TK / cmssw

Fork of CMSSW where improvements to L1 tracking code are developed.
http://cms-sw.github.io/
Apache License 2.0
4 stars 5 forks source link

Rearrange newKF chain #281

Closed cgsavard closed 2 months ago

cgsavard commented 3 months ago

PR description:

This PR removes the ProducerTT and ProducerAS modules and instead converts the KF tracks to TTTracks in the KFout module. This was necessary in order to fill the track quality MVA variables in the TTTracks which is calculated in the KFout. This also removes AnalyzerTT as a result.

old chain: KF -> TT -> AS -> KFout new chain: KF -> KFout (conversion to tttracks done here)

PR validation:

The code checks have been run and I have checked that the results of the KFout module before and after the changes made match.

Chriisbrown commented 2 months ago

image

The track MVA1 is looking off when running the L1TrackNtupleMaker with hybrid_newkf

cgsavard commented 2 months ago

The track MVA1 is looking off when running the L1TrackNtupleMaker with hybrid_newkf

This is expected since the chi2 variables are not correct. They are not /dof and so their values is larger than expected, and therefore we expect things to look more fake (will be fixed in a separate PR).

cgsavard commented 2 months ago

@tschuh @Chriisbrown @tomalin I attempted to merge but all of a sudden it looks like the merged commit no longer passes the git checks (it fails at setup_cmssw https://github.com/cms-L1TK/cmssw/commit/5cf9b2740211b935be9db4bab841b37e457b83d4). Can someone more knowledgable on the git checks advise on what should be done here? Should I revert the merge for now?

tschuh commented 2 months ago

Well, it says that it got successfully merged. Fresh checkout also compiles. I think everything is ok.

tomalin commented 2 months ago

The track MVA1 is looking off when running the L1TrackNtupleMaker with hybrid_newkf

This is expected since the chi2 variables are not correct. They are not /dof and so their values is larger than expected, and therefore we expect things to look more fake (will be fixed in a separate PR).

Anyone know why the chi2 variables are not correct with hybrid_newkf? Claire implies they are not normalized to dof. Why not?

Chriisbrown commented 2 months ago

The track MVA1 is looking off when running the L1TrackNtupleMaker with hybrid_newkf

This is expected since the chi2 variables are not correct. They are not /dof and so their values is larger than expected, and therefore we expect things to look more fake (will be fixed in a separate PR).

Anyone know why the chi2 variables are not correct with hybrid_newkf? Claire implies they are not normalized to dof. Why not?

When the firmware and emulation was originally written I was going off the digitization format of the twiki that doesn't mention /dof and has an older binning, the more recent interface document does give /dof and a revised binning so the emulation and firmware need to be updated to the /dof version. As previously to this PR the track word wasn't filled by the KF out and this non-dof version of the chi2 it didn't impact anything downstream, now that it does fill the trackword this needs fixing. It was decided that this would be a separate PR and that the chi2 values and MVA values will be wrong until that fix is done