star-bnl / star-sw

Core software for STAR experiment
26 stars 63 forks source link

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

Closed jdbrice closed 1 month ago

jdbrice commented 4 months ago

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

Changes:

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 commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

Thanks for the replies, Daniel. You are right that others have made such configuration files. It's not forbidden, but I really don't think it buys you anything that the database table scheme doesn't already offer...

I understand that you don't want to ask @dmarkh to implement a database table on the database servers while you're still settling in on what fields need to be in the configuration, but that's accounted for by using CINT files (under StarDb) with a table definition (StDb/idl) in the meantime....though admittedly that requires a re-compile each time you change the definition of the fields. Of course it doesn't hurt anything if you put parameters that may never change into that table as well....just in case they ever do.

And if you're going to implement the capability to use the database later, you've created more work for yourself by using a configuration file now. But it's too late for that argument, I suppose, since that work has already been done. In the end, if switching to database tables now is going to slow progress, then I won't stand in the way.

-Gene

On Dec 19, 2023, at 9:04 AM, Daniel Brandenburg @.***> wrote:

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

• 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. • 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.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

genevb commented 4 months ago

Well, one caveat that just came to mind....

One can of course still decide which CINT files to keep under the StarDb directory as a means of selection.

-Gene

jdbrice commented 4 months ago

@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.