Skip to content

Conversation

@jdbrice
Copy link
Contributor

@jdbrice jdbrice commented Jul 30, 2025

  • PicoVtxless: sets the PicoDst vertex mode to "Vtxless" - to mirror update in code
  • fttSim: Run the ftt (fast) sim
  • FcsTrackMatch: run the Fwd track to FCS cluster matching

Copy link
Contributor

@klendathu2k klendathu2k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

{"FttQA","","fttChain","","StFttQAMaker","StFttQAMaker", "sTGC Raw hit QA maker", kFALSE},

{"FwdTrack","","","fcsDb","StFwdTrackMaker",
"XMLIO,genfit2,KiTrack,StarGeneratorUtil,libMathMore,StEventUtilities,StEpdUtil,StFwdTrackMaker",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] [off topic]. Could you comment on why the tracker depends on event generator utilities?

Copy link
Contributor

@genevb genevb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK to me as well, but would also like to hear about the event generator dependence @klendathu2k asked.

{"PicoVtxVpdOrDefault","","","-PicoVtxDefault" ,"","","pico Vtx cut on Tof and VPD or default",kFALSE},
{"PicoVtxFXT" ,"","","-PicoVtxDefault" ,"" ,"","pico Vtx constraint on FXT [198,202] mode",kFALSE},
{"PicoVtxMtd" ,"","","-PicoVtxDefault" ,"" ,"","pico Vtx using MTD matching mode",kFALSE},
{"PicoVtxless" ,"","","-PicoVtxDefault" ,"" ,"","pico Vtx NOT required for FWD" ,kFALSE},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{"PicoVtxless" ,"","","-PicoVtxDefault" ,"" ,"","pico Vtx NOT required for FWD" ,kFALSE},
{"PicoVtxless" ,"","","-PicoVtxDefault" ,"" ,"","pico Vtx NOT required for FWD",kFALSE},

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, don't we need a concurrent change in StBFChain.cxx to pass this along to StPicoDstMaker?

else if ( GetOption("PicoVtxless"))      mk->SetAttr("PicoVtxMode", "PicoVtxless");

...near line 698 of StBFChain.cxx.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in #639

{"PicoVtxVpdOrDefault","","","-PicoVtxDefault" ,"","","pico Vtx cut on Tof and VPD or default",kFALSE},
{"PicoVtxFXT" ,"","","-PicoVtxDefault" ,"" ,"","pico Vtx constraint on FXT [198,202] mode",kFALSE},
{"PicoVtxMtd" ,"","","-PicoVtxDefault" ,"" ,"","pico Vtx using MTD matching mode",kFALSE},
{"PicoVtxless" ,"","","-PicoVtxDefault" ,"" ,"","pico Vtx NOT required for FWD" ,kFALSE},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, don't we need a concurrent change in StBFChain.cxx to pass this along to StPicoDstMaker?

else if ( GetOption("PicoVtxless"))      mk->SetAttr("PicoVtxMode", "PicoVtxless");

...near line 698 of StBFChain.cxx.

@genevb genevb added the FWD Forward Upgrade label Aug 28, 2025
Copilot AI review requested due to automatic review settings August 28, 2025 17:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds three new BFC (BigFullChain) options to support Forward (FWD) detector productions, including fast simulation capabilities and track matching functionality.

  • Adds fttSim option to enable FTT (Forward Tracking Telescope) fast simulation
  • Adds FcsTrackMatch option for matching forward tracks to FCS (Forward Calorimeter System) clusters
  • Adds PicoVtxless option to disable vertex requirements for forward detector analysis

Comment on lines +1710 to +1711
{"fttSim","fttChain","","StEvent,fttDb",
"StFttFastSimulatorMaker","StFttFastSimulatorMaker","Ftt Fast Simulator", kFALSE},
Copy link

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The description 'Ftt Fast Simulator' should use consistent capitalization with other entries. Consider 'FTT Fast Simulator' to match the naming convention used elsewhere in the file.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +1726 to +1727
{"FcsTrackMatch","","","FwdTrack","StFcsTrackMatchMaker",
"StFcsTrackMatchMaker",
Copy link

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The formatting is inconsistent with other entries. The second line should be aligned with the opening quote of the first line to maintain consistent indentation style throughout the file.

Copilot generated this review using guidance from repository custom instructions.
@fgeurts
Copy link
Member

fgeurts commented Sep 16, 2025

Is this pull request still actual? Or has it been split across other PRs? Either way, besides "co-pilot", are there any aspects that need follow-up before closing/merging?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

FWD Forward Upgrade

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants