Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to StFwdTrackMaker to allow running without XML config file #635

Merged
merged 5 commits into from
Mar 27, 2024

Conversation

jdbrice
Copy link
Contributor

@jdbrice jdbrice commented Dec 15, 2023

This update makes the StFwdTrackMaker fully configurable from the ROOT macro without the need for any external XML config file.

Changes:

  • FwdTrackerConfig: new functionality to allow loading a config from a string (instead of a file) and new functions for setting arbitrary values in the config
  • StFwdTrackMaker: many new functions for setting configuration parameters
  • New logic to use a default configuration if set for Data or Mc, else load from a config file

The default config file is embedded for consistency.
In the future BFC options can be used to toggle parameters if they need to change from one production to the next.

@genevb
Copy link
Contributor

genevb commented Dec 18, 2023

My primary question is...
Why not handle this configuration with a database table? We already have so much capability built there already, including CINT file functionality for local configuration files.

We have strived to avoid specialized configuration files for production jobs, and while there is the possibility to determine some settings from BFC options, handling value-setting like setPrimaryVertexSigmaZ(double sZ) via BFC options requires even more customization.

Is there an argument against using a database table?

Thanks,
-Gene

@jdbrice
Copy link
Contributor Author

jdbrice commented Dec 19, 2023

@genevb
Hi Gene, it is a good point. There are two parts to the answer:

  1. This configuration system is made for running the code/tracking with different parameters for optimization and calibration - not for production options. In several of the other detector sub-systems we do have a text based config/parameter file for configuration. This is just a bit more robust/generic.
  2. In some cases studies in 1 will yield optimal values for production -> e.g. once we have identified the ideal track finding parameters for p+p, Au+Au then these should be stored in the database. At that time it will be easy for me to add a LoadConfigurationFromDatabase().

The reason I have not added configuration from the database yet is because we just introduced the new FST-first tracking mode and it is still in development - I am not even sure yet which parameters will be fixed and which should change from one production to another.

@genevb
Copy link
Contributor

genevb commented Dec 20, 2023 via email

@genevb
Copy link
Contributor

genevb commented Dec 20, 2023 via email

@jdbrice
Copy link
Contributor Author

jdbrice commented Dec 20, 2023

@genevb I want to clarify also that the config has been part of the FWD tracker since the beginning. This update just makes the FWD code use/need it LESS - so I agree with your comments, and as we move the FwdTracker to production ready we will adopt the database more and more. It probably was more work, but at the same time I have run the tracking code local (not even in STAR env) for a long time as I have developed it, and this has helped to that end.

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.

Marking this as reviewed: I don't object to moving this forward.

Copy link
Member

@plexoos plexoos left a comment

Choose a reason for hiding this comment

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

well documented

@jdbrice jdbrice merged commit 2e09826 into star-bnl:main Mar 27, 2024
148 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants