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

Update documentation etc. to use uenv start --view instead of uenv view #186

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

msimberg
Copy link
Contributor

@msimberg msimberg commented Feb 5, 2025

No description provided.

@msimberg msimberg requested review from bcumming, jgphpc and RMeli February 5, 2025 11:21
Comment on lines 43 to 44
The `/user-tools/activate` script will make the forge executables available in your environment, and **must be run before**
any other uenv view command otherwise the environment variables set from the other uenv will be overwritten.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jgphpc could you clarify what the requirements are here? Is the /user-tools/activate script not going to work correctly with the view loaded at start? Is it a requirement to source the activate script?

Copy link
Contributor

Choose a reason for hiding this comment

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

At the time of doc writing, this was the only way but now, loading 2 views is working fine:

uenv start prgenv-gnu/24.11:v1,linaro-forge/24.0.2:v1 -v forge,prgenv-gnu:default
uenv start prgenv-gnu/24.11:v1,e0e79f5c3e6a8ee0 -v forge,prgenv-gnu:default

which nvcc
/user-environment/env/default/bin/nvcc

which ddt
/user-tools/env/forge/bin/ddt

so it's fine to remove that part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @jgphpc! I've updated this to load both uenvs and views in one command. Please have a look to see if it's sane to you.

docs/uenv-prgenv-gnu.md Outdated Show resolved Hide resolved
Comment on lines 11 to 12
# TODO: Update this and other reframe activations?
activation: uenv view develop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there someone who knows if these activations still work with the new uenv? Does reframe need changes to support loading a view at uenv start-time?

@ekouts you might know best what's supported/required?

Copy link
Member

Choose a reason for hiding this comment

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

Reframe has support for loading views at start time.
There is a view parameter that can be configured in the extra/reframe.yaml file that specifies which view(s) to load.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@msimberg msimberg Feb 10, 2025

Choose a reason for hiding this comment

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

Thanks. Updated (all the prgenv-gnu recipes). If I understand correctly, these recipes would need to be rebuilt and deployed with a new version to be able to use the updated reframe.yaml files, yes? (Though all the recipes older than 24.11 probably don't benefit from a rebuild.)

From my perspective this can wait for the next version bump of the prgenv-gnu uenvs, though it wouldn't hurt to have a prgenv-gnu which works with the new uenv. Feels a bit strange to bump it to v2 with a non-user-facing change, but maybe it doesn't matter. Preferences?

@msimberg msimberg force-pushed the uenv-view-docs branch 5 times, most recently from e77ccdc to 0e44fff Compare February 10, 2025 13:58
@msimberg msimberg marked this pull request as ready for review February 12, 2025 18:43
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.

3 participants