star-bnl / star-sw

Core software for STAR experiment
26 stars 63 forks source link

Add makers and utilities for FST hit processing #266

Closed jdbrice closed 2 years ago

jdbrice commented 2 years ago

FST additions for offline reconstruction chain - builds on PR#265. Adds Chain options: "fstDb" - FST Db Maker "fstFastSim" - should have been added before, adding now for completeness "fstRawHit" - FST Raw hit (from DAQ) maker "fstCluster" - StFstClusterMaker "fstHit" - StFstHitMaker "fstUtil" - Loads the Fst Utilities

Test chains (RawHitMaker, populates base StEvent structures): "in,fstDb,fstRawHit" data file here: /star/data01/pwg/yezhenyu/FTS/Shenghui/Offline/data/st_fstcosmic_22340057_raw_0000010.daq

Test chain (reco): "in,fstUtil,fstRawHit,fstDb,fstCluster,fstHit" /star/u/sunxuhit/ForwardSiliconTracker/Data/FstInstallation/daqtest/351/st_physics_22351023_raw_1500001.daq

starsdong commented 2 years ago

Hi Daniel,

Just to clarify, the StEvent relevant containers are the same as in PR#265, correct? Similar as the FCS software review, maybe I would suggest to ask colleagues to split the review into two parts

1) FstDbMaker and FstRawHitMaker

2) FstClusterMaker, FstHitMaker and FstUtil

In addition, there will be the BFC chain options which Gene has commented on.

For 1), I would suggest to include Dmitry A. and maybe ask Hongwei to help review. For 2), I would suggest to ask maybe Flemming and Grigory to help?

What do you think? Thanks

/xin

dmarkh commented 2 years ago

FST database:

StFstDbMaker/StFstDb: Please do not store pointers to structs if you don't own those. These are temporary and may be invalidated at any moment by the St_db_Maker. If you want to store that data reliably long-term, then please copy data into your own structures via GetTableCpy():

...first, get a copy of data... mFstDb->setGain(mGain->GetTable()); => mFstDb->setGain(mGain->GetTableCpy());

..then, cleanup old data before the update... void setGain(fstGain_st gain) {mFstGain = gain;} => void setGain(fstGain_st gain) { free(mFstGain); mFstGain = gain;} // because calloc in GetTableCpy

... then repeat for all Calibrations/fst/... tables. Geometry/fst is okay as data is being transformed and pointers are not stored by the fstDb object.

dmarkh commented 2 years ago

Okay, I have created the remaining Geometry/fst tables as per request: https://drupal.star.bnl.gov/STAR/blog/yezhenyu/FST-Offline-Database

Zhenyu, please proceed with data upload. Examples could be found under: https://online.star.bnl.gov/dbExplorer/

Database initialization timestamps for Run22 could be found here: https://chat.sdcc.bnl.gov/star/pl/z153ij6tcfr3bpo66b99uywxoy

sunxuhit commented 2 years ago

Okay, I have created the remaining Geometry/fst tables as per request: https://drupal.star.bnl.gov/STAR/blog/yezhenyu/FST-Offline-Database

Zhenyu, please proceed with data upload. Examples could be found under: https://online.star.bnl.gov/dbExplorer/

Database initialization timestamps for Run22 could be found here: https://chat.sdcc.bnl.gov/star/pl/z153ij6tcfr3bpo66b99uywxoy

Updated the StFstDbMaker as suggested.

The Calibrations/fst/ is currently empty, and we will start to upload data with daily calibration files and other databases. I hope this shouldn’t prevent this PR to be merged.

The data table under Geometry/fst/ are still under development. To complete, the survey data needs to be correctly put in for FST. The purpose for this PR is to start the fast offline QA so FST data got monitored asap. We are using some hard-coded ideal geometry for now and transfer to the database later.

dmarkh commented 2 years ago

Regarding empty calibrations tables - the StFstMaker code will abort if no data is found in the database. STAR requires subsystem experts to upload initial "ideal" calibrations using predefined timestamps before the data taking to avoid chain crashes.

Data upload is easy, examples are provided for each table at: https://online.star.bnl.gov/dbExplorer/ Calibrations -> FST -> reconV0 ->

jdbrice commented 2 years ago

I propose an "fstChain" that runs "fstRawHit,fstCluster,fstHit". @jdbrice , either you can do it by mimicking what I did for "fcsChain", or I can provide you with a version of BigFullChain.h with that in place (it's not complicated, so I don't mind doing it).

Otherwise the BFC stuff looks generally OK.

-Gene

@genevb I added this in commit 068a89f1c443a08285c15a967de32e57c85a5dcf

please check that it is done as you expect.

sunxuhit commented 2 years ago

Regarding empty calibrations tables - the StFstMaker code will abort if no data is found in the database. STAR requires subsystem experts to upload initial "ideal" calibrations using predefined timestamps before the data taking to avoid chain crashes.

Data upload is easy, examples are provided for each table at: https://online.star.bnl.gov/dbExplorer/ Calibrations -> FST -> reconV0 ->

The Calibration tables are all uploaded, at least the initial version of them. The Geometry tables are still under development, and will be uploaded in a later time (not in this PR). I hope the current database is good enough for a test run. I will clean up the macro and upload all the pedestal files to database in final format later after I am fully confident to do so.

dmarkh commented 2 years ago

The Calibration tables are all uploaded, at least the initial version of them. The Geometry tables are still under development, and will be uploaded in a later time (not in this PR). I hope the current database is good enough for a test run. I will clean up the macro and upload all the pedestal files to database in final format later after I am fully confident to do so.

Regarding failing tests - my bad, sorry! The GetTable call comes from the TTable macro, not from StDbTable as I originally thought.

So, please revert GetTableCpy() calls back to the original GetTable(), and remove free() calls.

Regarding uploaded tables - you and I agreed that you will tell me which tables are temporary and should be deleted. Does your reply mean that all tables are okay and nothing needs to be deleted?

sunxuhit commented 2 years ago

The Calibration tables are all uploaded, at least the initial version of them. The Geometry tables are still under development, and will be uploaded in a later time (not in this PR). I hope the current database is good enough for a test run. I will clean up the macro and upload all the pedestal files to database in final format later after I am fully confident to do so.

Regarding failing tests - my bad, sorry! The GetTable call comes from the TTable macro, not from StDbTable as I originally thought.

So, please revert GetTableCpy() calls back to the original GetTable(), and remove free() calls.

Regarding uploaded tables - you and I agreed that you will tell me which tables are temporary and should be deleted. Does your reply mean that all tables are okay and nothing needs to be deleted?

All the GetTableCpy() are switched by to GetTable(), and removed all the free command.

For the tables, those are still temporary. I was wondering is that possible to run some test production with the temporary, so I could adjust accordingly if anything is wrong. If we need the final table for the test run, I will double check my macro and update the final ones asap.

starsdong commented 2 years ago

Hongwei, do you have any further comment on the RawHitMaker and DbMaker?

Flemming and Grigory, would be great if you can also take a review of the new commits and comment/approve?

Thanks

fvidebaek commented 2 years ago

I will get it done most likely tomorrow morning.

-F

On 2022-01-17 12:13, Xin Dong wrote:

Hongwei, do you have any further comment on the RawHitMaker and DbMaker?

Flemming and Grigory, would be great if you can also take a review of the new commits and comment/approve?

Thanks

-- Reply to this email directly, view it on GitHub [1], or unsubscribe [2]. Triage notifications on the go with GitHub Mobile for iOS [3] or Android [4]. You are receiving this because you were mentioned.Message ID: @.***>

Links:

[1] https://github.com/star-bnl/star-sw/pull/266#issuecomment-1014748935 [2] https://github.com/notifications/unsubscribe-auth/AUIALBBWCN5AYH56TQA3HT3UWRE4JANCNFSM5KO2AAEQ [3] https://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!P4SdNyxKAPE!R8Bad9-bfz7bf8szG1iiHTV7NyUYDS2mrvs0kaxZOXZtqAx7WGJdDjJGvFw1VZ2q$ [4] https://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign*3Dnotification-email*26utm_medium*3Demail*26utm_source*3Dgithub__;JSUlJSU!!P4SdNyxKAPE!R8Bad9-bfz7bf8szG1iiHTV7NyUYDS2mrvs0kaxZOXZtqAx7WGJdDjJGvFxDD8My$

-- Flemming Videbaek senior scientist videbaek @ bnl.gov Brookhaven National Lab Physics Department Bldg 510D Upton, NY 11973

phone: 631-344-4106 cell : 631-681-1596

starsdong commented 2 years ago

Thank you Flemming and Hongwei for the approvals.

Gene, are you OK with the chain options? If all OK, I think we can merge this PR into main. Thank you

genevb commented 2 years ago

Gene, are you OK with the chain options? If all OK, I think we can merge this PR into main. Thank you

No. My corrections to BigFullChain.h have not been addressed.

Thanks, -Gene

jdbrice commented 2 years ago

@genevb commit 9b5c35b33c9704215d0af043679bb1fb44e8fab8 addresses the issues with the BFC. Please let me know if any other changes are needed

starsdong commented 2 years ago

Dmitri, the review process for these new makers + chain options is complete. Let us merge this PR into main.

Thanks