Jet tree maker updates with additional TOF information saved in the trees

[zlchang]

Hi @plexoos,

I have removed the pointless comments "//bassam". Those were added by Bassam to better distinguish his edits. I will relay this message to him so that these line won't be needed.

I removed the FMS tower in the StJetMaker2012, this is because we expect a new jet maker to be uploaded that is specifically written for FMS jet analysis by Latif Kabir soon.

ClassDef(StJetMaker2009, 0) is kept because this maker is not for ROOT I/O, previously Bassam incremented it by 1, and I changed it back to 0, that's why you saw "zchang" prefixed to original Bassam's comments. I removed the comment as well.

This pointer has been used in our jet code extensively before. It may be better to keep it as it is now to avoid modifying it everywhere in the code. Let me know if this is okay with you.

Indentation is wrong in multiple places even from the original code, but I fixed those indentations you mentioned in StJetSkimMaker and StJetSkimEvent.

[zlchang]

Hi @plexoos,

I just committed an updates, but it seems the ROOT5 compilation check has failed. I can compile the code on RCF under SL21a.

[plexoos]

Thanks for cleaning up the code Zilong. It looks better and I don't have other suggestions. The issue with the ROOT5 build appears to be a glitch on the GitHub side and is not related to your changes. If you go to the "Checks" tab you should see a "Re-run jobs" button. Let's see if the jobs go through this time.

[zlchang]

@akioogawa and @veprbl any comments for this code, and we need this for the run17 jet tree production?

[veprbl]

Why PR that is supposed to add TOF information is removing FMS?

[zlchang]

There will be another maker dedicated to FMS jet analysis from Latif uploaded soon.

On Aug 9, 2021, at 9:14 AM, Dmitry Kalinkin @.***> wrote:



Why PR that is supposed to add TOF information is removing FMS?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/star-bnl/star-sw/pull/79#issuecomment-895212445, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGH7F3MHAL5P5KYGAO4FFZTT37IDJANCNFSM5BM25RFA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.

[veprbl]

Latif can remove FMS then :)

[zlchang]

I discussed this with Latif and we both agreed that we can remove FMS in the jet maker and there is also a bug for the EEMC tower when no vertex found and we will update it when his code is ready.

Sent from my iPhone

On Aug 9, 2021, at 9:40 AM, Dmitry Kalinkin @.***> wrote:



Latif can remove FMS then :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/star-bnl/star-sw/pull/79#issuecomment-895232059, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGH7F3MLGL4GI56VKLDXKWTT37LFTANCNFSM5BM25RFA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.

[veprbl]

So is FMS being removed or not? There are still files left:

# find StRoot/StJetMaker -iname "*FMS*"                         
StRoot/StJetMaker/towers/StjFMSTree.h
StRoot/StJetMaker/towers/StjFMS.h
StRoot/StJetMaker/towers/StjFMSNull.h
StRoot/StJetMaker/towers/StjFMSTree.cxx
StRoot/StJetMaker/towers/StjFMSNull.cxx
StRoot/StJetMaker/towers/StjFMS.cxx
StRoot/StJetMaker/mudst/StjFMSMuDst.cxx
StRoot/StJetMaker/mudst/StjFMSMuDst.h

https://github.com/star-bnl/star-sw/blob/3d486c7bfa5074a0dff650109391414fc75a9f83/StRoot/StJetMaker/StAnaPars.h#L108 https://github.com/star-bnl/star-sw/blob/3d486c7bfa5074a0dff650109391414fc75a9f83/StRoot/StJetMaker/macros/RunJetFinder2012pro.C#L46-L53 https://github.com/star-bnl/star-sw/blob/3d486c7bfa5074a0dff650109391414fc75a9f83/StRoot/StJetMaker/macros/RunJetFinder2012pro.C#L80-L83

[zlchang]

These will be used in his new maker. The jet maker is just to read the inputs from MuDst files and call the jet finder. His new maker is very similar to the current StJetMaker2012.

On Aug 9, 2021, at 10:33 AM, Dmitry Kalinkin @.***> wrote:



So is FMS being removed or not? There are still files left:

find StRoot/StJetMaker -iname "FMS"

StRoot/StJetMaker/towers/StjFMSTree.h StRoot/StJetMaker/towers/StjFMS.h StRoot/StJetMaker/towers/StjFMSNull.h StRoot/StJetMaker/towers/StjFMSTree.cxx StRoot/StJetMaker/towers/StjFMSNull.cxx StRoot/StJetMaker/towers/StjFMS.cxx StRoot/StJetMaker/mudst/StjFMSMuDst.cxx StRoot/StJetMaker/mudst/StjFMSMuDst.h

https://github.com/star-bnl/star-sw/blob/3d486c7bfa5074a0dff650109391414fc75a9f83/StRoot/StJetMaker/StAnaPars.h#L108 https://github.com/star-bnl/star-sw/blob/3d486c7bfa5074a0dff650109391414fc75a9f83/StRoot/StJetMaker/macros/RunJetFinder2012pro.C#L46-L53 https://github.com/star-bnl/star-sw/blob/3d486c7bfa5074a0dff650109391414fc75a9f83/StRoot/StJetMaker/macros/RunJetFinder2012pro.C#L80-L83

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/star-bnl/star-sw/pull/79#issuecomment-895275593, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGH7F3IZS74ZMBFAD2I6UCDT37RK5ANCNFSM5BM25RFA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.

[plexoos]

Zilong, Dmitry makes a good point asking you to not submit changes unrelated to the topic stated in this PR. Just create a separate PR with FMS modifications, it costs nothing but makes the intention much clear.

[veprbl]

Zilong, Dmitry makes a good point asking you to not submit changes unrelated to the topic stated in this PR. Just create a separate PR with FMS modifications, it costs nothing but makes the intention much clear.

My standard is lower than that. I'm asked to review change in context of some other change that I haven't seen. If there exists a privately discussed consensus between jet experts, I don't mind this merged, just let's not pretend that I've reviewed this.

[zlchang]

That’s easy to do and I can put these changes back in.

On Aug 9, 2021, at 10:45 AM, Dmitri Smirnov @.***> wrote:



Zilong, Dmitry makes a good point asking you to not submit changes unrelated to the topic stated in this PR. Just create a separate PR with FMS modifications, it costs nothing but makes the intention much clear.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/star-bnl/star-sw/pull/79#issuecomment-895285234, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGH7F3KRW4B3WLOHADI5BE3T37SZ5ANCNFSM5BM25RFA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.

[zlchang]

There is nothing wrong with this code, it works and it has already been tested.

On Aug 9, 2021, at 11:08 AM, Dmitry Kalinkin @.***> wrote:



Zilong, Dmitry makes a good point asking you to not submit changes unrelated to the topic stated in this PR. Just create a separate PR with FMS modifications, it costs nothing but makes the intention much clear.

My standard is lower than that. I'm asked to review change in context of some other change that I haven't seen. If there exists a privately discussed consensus between jet experts, I don't mind this merged, just let's not pretend that I've reviewed this.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/star-bnl/star-sw/pull/79#issuecomment-895304794, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGH7F3OT6AI54VIS7QOSG5DT37VPBANCNFSM5BM25RFA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.

[zlchang]

I pushed a new set of commit, let me know if this is okay with you guys.

[BassamAboona]

Hi Zilong! The edits related to TOF information look good to me. Thank you very much for making these edits!

[zlchang]

Are we ready to move forward with this pull request?

[plexoos]

When merging this PR please use "Squash and merge" in order to keep the main branch clean