star-bnl / star-sw

Core software for STAR experiment
26 stars 63 forks source link

Create PWGTools area #340

Closed nigmatkulov closed 1 year ago

nigmatkulov commented 2 years ago

The commit will create a new directory that should be excluded from the compilation process. At the first iteration it should be cross checked (if compilation will pass the CI/CD). Also before merging Zachary Sweger should confirm that the subdirectories contain the latest versions of the Glauber codes.

veprbl commented 2 years ago

A question about naming. Does "PWG" stand for "Physics Working Group"? Which one is referred to? Also, if this is a collection of libraries, why it doesn't follow the standard St*Pool naming?

nigmatkulov commented 2 years ago

A question about naming. Does "PWG" stand for "Physics Working Group"? Which one is referred to? To all PWGs.

Also, if this is a collection of libraries, why it doesn't follow the standard St*Pool naming? I think it is doable, but idea was to create an area where PWGs could store all physics-related codes, not software/hardware software which used for the library builds. At some point it may be deployed from StRoot.

plexoos commented 2 years ago

Would it make sense to keep these files in a separate repository? I thought that idea was discussed in the meetings but I don't remember the arguments against it

marrbnl commented 2 years ago

Originally, I was worried about synchronization to RCF if in a separate repository. Thinking more on this, putting these codes in a separate repository does come with the benefit of decoupling from StRoot, so it might be easier to manage and work with. The downside is that people need to download it from Git as it won't be available on RCF, if I understand correctly

plexoos commented 2 years ago

I believe what you really want to share on the SDCC machines is the libraries built from the source code rather than the source code itself. But if the code cannot be compiled then there will be no libraries to share. I understand there are some scripts to run jobs that can be also shared, but again, installing them without something they can use wouldn't make sense either.

marrbnl commented 2 years ago

I was actually thinking of sharing the source codes and running scripts, basically the whole package. I think I am used to CVS, where I just copy over the source codes. In any case, I am fine with a separate repository

marrbnl commented 2 years ago

Hello. To follow up on this, what is the next step?

veprbl commented 2 years ago

If I were a reviewer, I would suggest:

marrbnl commented 2 years ago

I prefer the name PWGTools since more codes could be saved in this directory, e.g. codes used to evaluate tracking efficiency uncertainty, etc. Meanwhile, I agree that we should not commit duplicated codes, and a dedicated macro directory sounds like a good idea.

Thinking about how we might use this directory in the future, a separate repository does make sense.

plexoos commented 2 years ago
marrbnl commented 2 years ago

I think a repository named star-pwg-tools under star-bnl would do.

plexoos commented 2 years ago

Done.

plexoos commented 1 year ago

Closing because star-pwg-tools will be used for this purpose

veprbl commented 1 year ago

I think that the creation of side repo under star namespace is unjustified in this case. It has no code review policy or CI in place, and the only apparent reason for its existence is to bypass those requirements.

marrbnl commented 1 year ago

The main motivation when I suggested to create a separate repository is not to interfere with StRoot, which I always thought is for codes needed for production. The codes we would like to add are more analysis specific with lots of macros and scripts. There was no intention at all to bypass any CI test or code review. If people think it is better to add it to StRoot, that is also fine with me.

marrbnl commented 1 year ago

I think the key is to make a decision, and move ahead.

veprbl commented 1 year ago

The main motivation when I suggested to create a separate repository is not to interfere with StRoot, which I always thought is for codes needed for production. The codes we would like to add are more analysis specific with lots of macros and scripts. There was no intention at all to bypass any CI test or code review. If people think it is better to add it to StRoot, that is also fine with me.

This is not a question of the motivation. I am only predicting the outcome of creating such separate repo under such circumstance - it will be an absence of the CI and there won't be much code review. The multirepo approach is not forbidden in principle, but it does require some extra dedication to maintain, and it takes more mental effort to track all the different versions of various codes. That's one argument against segregating from the "production" codes, for which all the procedures are already in place. It also should make sense to have all core functionality codes such as reconstruction, calibration and common physics tools in one place where they are most seen by the software experts. Plus, they all serve the same goal of enabling wide range of collaborators to do their best with our data.

nigmatkulov commented 1 year ago

Responding to the requested review as a "code owner":

I'm not a supporter of adding yet another directory under StRoot that is specifically excluded from compilation, but I won't block this effort as I think it's critical to stop the proliferation of individual private repositories that the use of git in STAR has (expectedly) invigorated. We will just add PWGTools to the SKIP_DIRS list of such directories.

(small addendum: Dmitri's commit to change cons so that it excludes PWGTools only applies when SKIP_DIRS is not defined, but for official library compiles we do indeed have SKIP_DIRS defined, so the exclusion will be via SKIP_DIRS for such compiles.)

Hi Gene, I believe that it is still an intent to make the codes be a part of compilation. I also dislike the idea of codes being compiled. I'm still trying to figure out what is causing the compilation breaks during the builds. I have an idea but need to test it first.

veprbl commented 1 year ago

Can we rename PWGTools -> StCentralityPool ?

marrbnl commented 1 year ago

This PWGTools directory is intended to host common analysis related codes. I suggest not to change the name. If desired, one change CentralityCalibration to StCentralityPool

plexoos commented 1 year ago

Ok, let's keep the originally proposed name. If no more immediate comments or additions, we can merge.

genevb commented 1 year ago

I can imagine that it is probably worth associating specific versions of PWGTools codes with specific library versions (there may, for example, be compilation dependencies), so I will deploy the current version of PWGTools when I release a new library, such as SL23b. I am wondering if people foresee updating libraries like SL23b when updated versions of PWGTools codes are developed?

Example: Let's say that SL23b is the last library to have some particular variable in the PicoDst, replaced by some better means of determining some physics in SL23c. People analyzing PicoDsts produced in SL23b will need to use the PWGTools codes that have the old variable, so they won't want the latest version of PWGTools from DEV. But it isn't until 4 months after producing data with SL23b that analyzers of that data finish their adjustments to relevant codes in PWGTools. Will we then re-tag a new version of SL23b with the updated PWGTools and deploy that to AFS and CVMFS? Such a deployment isn't a big deal since we won't actually need to recompile anything in the official libraries on AFS and CVMFS. But I would like to understand if this will be the expected procedure, or whether analyzers will be expected to pull the version of PWGTools they are expected to use from github themselves (in which case, why deploy PWGTools in any AFS or CVMFS libraries from the start)?