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

Add scripts to load dependency packages via Environment Modules #409

Merged
merged 6 commits into from
Oct 25, 2022

Conversation

jdbrice
Copy link
Contributor

@jdbrice jdbrice commented Oct 12, 2022

This PR provides 2 configs to load spack library versions:
0.2.3 (32b & 64b)

I have not added scripts for root 6 variants yet as I do not think these are used by anyone.
If I understand correctly, these will become usable with, e.g.:

starver v0.2.3-rhel7-root5.34.38-32b

OR

starver v0.2.3-rhel7-root5.34.38-64b

Closes #359

@klendathu2k
Copy link
Contributor

Did you mean to write starver dev v0.2.0-5.34.38?

Or is there a star script (sitting somewhere out of my search path) which takes the option ver?

@plexoos
Copy link
Member

plexoos commented Oct 12, 2022

Did you mean to write starver dev v0.2.0-5.34.38?

Yes, it should be starver (no space) as explained in this comment

@klendathu2k
Copy link
Contributor

Did you mean to write starver dev v0.2.0-5.34.38?

Yes, it should be starver (no space) as explained in this comment

Thanks. Also in that comment (and reading the code...) starver expects a filename like v0.2.0-5.34.38-64b.config not v0.2.0-5.34.38-64b.csh. Or am I missing something?

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.

As noted in other comment... I believe these scripts should carry the ".config" suffix rather than ".csh".

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.

I am a bit concerned about the multiplicity of config files that could / will appear in mgr. I would propose to address this by organizing the scripts into subdirectories in mgr. The starver script already should support this...

starver dev v0.2.0/5.34.38/64b would execute mgr/v0.2.0/5.34.38/64b.config

There would need to be a directory layout naming convention...

star-environment/root-version/ ... /32b.config or 64b.config

Other subdirectories to be added for (e.g.) geant versioning.

@genevb
Copy link
Contributor

genevb commented Oct 12, 2022

There would need to be a directory layout naming convention...

star-environment/root-version/ ... /32b.config or 64b.config

Other subdirectories to be added for (e.g.) geant versioning.

I appreciate the suggestion to minimize clutter, @klendathu2k .Can we know in advance what all items need versioning (e.g. GEANT, then what else?)....because re-organizing later would be problematic for maintaining compatibility of people's private scripts.

@klendathu2k
Copy link
Contributor

I appreciate the suggestion to minimize clutter, @klendathu2k .Can we know in advance what all items need versioning (e.g. GEANT, then what else?)....because re-organizing later would be problematic for maintaining compatibility of people's private scripts.

@genevb Geant versioning would be good. But as we add more and more directory levels... the starver command becomes less and less user friendly. More error prone for users to discover the available scripts. More annoying to type. So perhaps a flat directory structure is preferred after all. But the scripts should be under mgr/config instead of mgr to avoid the clutter.

@klendathu2k
Copy link
Contributor

And one last comment. The starver script looks for these environment scripts under $STAR/mgr. If it isn't found, a default config is executed. This means there is no possibility to specify a local script. The trick that I normally use (setting STAR to the local directory) doesn't work... b/c starver resets it.

$ ls mgr/mylocal.config
mgr/mylocal.config
$ setenv STAR .
$ starver dev mylocalversion
$ echo $STAR
/afs/rhic.bnl.gov/star/packages/DEV

@jdbrice
Copy link
Contributor Author

jdbrice commented Oct 18, 2022

And one last comment. The starver script looks for these environment scripts under $STAR/mgr. If it isn't found, a default config is executed. This means there is no possibility to specify a local script. The trick that I normally use (setting STAR to the local directory) doesn't work... b/c starver resets it.

$ ls mgr/mylocal.config
mgr/mylocal.config
$ setenv STAR .
$ starver dev mylocalversion
$ echo $STAR
/afs/rhic.bnl.gov/star/packages/DEV

Right, Jerome mentioned at the beginning that he would add this if we requested. Maybe we should request, as it would also help test this before final integration

@jdbrice
Copy link
Contributor Author

jdbrice commented Oct 20, 2022

Ok I sent an email to Jerome to address the loading of configs from local user directories.
But we need to converge on the naming and backward compatibility issues as well.
Commit b9a24df moves them into config sub directory and attempts to name them in a verbose way - but this is not user friendly at all

@plexoos
Copy link
Member

plexoos commented Oct 20, 2022

I would drop the v0.2.0 configs and only keep the currently latest v0.2.3. Since it is going to be the first time we rely on this environment there is no need to keep the older ones around.

@jdbrice
Copy link
Contributor Author

jdbrice commented Oct 21, 2022

OK commit 35dd3b4 removes the v0.2.0 configs and removes geant3 from the name per @plexoos comments.

Also @plexoos - we talked about adding an alias as "default" or "current" etc. but does that need to be part of the PR?

Please let me know if if there are any other changes needed before approval

@jdbrice
Copy link
Contributor Author

jdbrice commented Oct 25, 2022

Hi can we resolve this - I have not received any new comments? We are waiting on this for the potential to run production with official libraries (or dev). Maybe just need an approval from @plexoos

I have not heard from Jerome, but will ping him.

@plexoos
Copy link
Member

plexoos commented Oct 25, 2022

I have no objections and can approve and merge ASAP. If I understand correctly this will not have an immediately effect as there is no mgr/config/default.config but it is probably a good thing at this point. It would be nice to hear from the production team

@plexoos plexoos changed the title Scripts to load spack libs Add scripts to load dependency packages via Environment Modules Oct 25, 2022
@plexoos plexoos merged commit b1f58b3 into star-bnl:main Oct 25, 2022
@genevb
Copy link
Contributor

genevb commented Oct 25, 2022

I also have no objection with these files in a config subdirectory, and no impact on official productions.

It would be nice to hear from the production team

Waiting only 19 minutes to merge after requesting to hear from the Production Team seems a bit....short? ;-)

-Gene

p.s. Jerome is at a conference for which he is an organizer and contributing author on multiple talks. I'm not saying whether or not you will hear from him, but I'm giving you some context for why you might not.

@plexoos
Copy link
Member

plexoos commented Oct 25, 2022

Waiting only 19 minutes to merge after requesting to hear from the Production Team seems a bit....short? ;-)

Depends on your reference point: 19 minutes since my comment or 13 days since the original post

Thank you for confirming the (lack of) possible impact!

@klendathu2k
Copy link
Contributor

I checked out the group directory from CVS and modified .starver to look for a local configuration file. I confirmed that my version of starver runs, and executes the configuration script. I expect that the environment will be setup for root 6. It still points to root 5. So... this does not appear ready for prime time.

This is the config file which will be executed...

$ cat mgr/v0.23-6.16.00.config 
#!/bin/csh -f
echo Setting spack env
module use /cvmfs/star.sdcc.bnl.gov/star-spack/spack/share/spack/modules/linux-rhel7-x86_64/
module load star-env-0.2.3-root-6.16.00
module load zlib-1.2.12
module load root-6.16.00
module list

This is the starver command executed and its result (I've deleted most of the module list output because of formatting on the git page...)

starver DEV v0.23-6.16.00
Setting spack env
Currently Loaded Modulefiles:
  1) /boost-1.59.0   ...    14) /root-6.16.00    ....

And now which version of ROOT is loaded?

$ which root.exe
/afs/rhic.bnl.gov/star/ROOT/5.34.38/.sl73_x8664_gcc485/rootdeb/bin/root.exe

@plexoos
Copy link
Member

plexoos commented Oct 26, 2022

I am a bit confused... What commands do I need to run to reproduce?

@klendathu2k
Copy link
Contributor

klendathu2k commented Oct 26, 2022 via email

@plexoos
Copy link
Member

plexoos commented Oct 26, 2022

Ok, I followed the steps exactly. Here is what I see:

$ which root.exe
/cvmfs/star.sdcc.bnl.gov/star-spack/spack/opt/spack/linux-rhel7-x86_64/gcc-4.8.5/root-6.16.00-bdclikaa3lfpfqolnfmwkcv5ws4vtepn/bin/root.exe

@klendathu2k
Copy link
Contributor

klendathu2k commented Oct 26, 2022 via email

@klendathu2k
Copy link
Contributor

klendathu2k commented Oct 26, 2022 via email

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.

Star dev Library: Include required spack libraries
5 participants