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 Cori base_url #54

Merged
merged 1 commit into from
Sep 1, 2022
Merged

Conversation

forsyth2
Copy link
Contributor

Update Cori base_url to use /cfs/ rather than /project/. Resolves #52.

@forsyth2 forsyth2 requested a review from xylar August 31, 2022 20:35
@forsyth2 forsyth2 self-assigned this Aug 31, 2022
Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

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

I tested this in MPAS-Analysis and the resulting URL was:

https://portal.nersc.gov/cfs/e3sm//xylar/analysis_testing/cori-haswell/mache_update_cori_base_url/main_py3.9

This works fine (try it) but it doesn't look nice so let's remove the unneeded trailing /.

@@ -34,7 +34,7 @@ group = e3sm
base_path = /global/cfs/cdirs/e3sm/www

# The base URL that corresponds to the base path
base_url = https://portal.nersc.gov/project/e3sm/
base_url = https://portal.nersc.gov/cfs/e3sm/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
base_url = https://portal.nersc.gov/cfs/e3sm/
base_url = https://portal.nersc.gov/cfs/e3sm

@@ -28,7 +28,7 @@ group = e3sm
base_path = /global/cfs/cdirs/e3sm/www

# The base URL that corresponds to the base path
base_url = https://portal.nersc.gov/project/e3sm/
base_url = https://portal.nersc.gov/cfs/e3sm/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
base_url = https://portal.nersc.gov/cfs/e3sm/
base_url = https://portal.nersc.gov/cfs/e3sm

@xylar
Copy link
Collaborator

xylar commented Sep 1, 2022

Results from my test can be found at:

/global/cscratch1/sd/xylar/analysis_testing/cori-haswell/mache_update_cori_base_url/main_py3.9
/global/cfs/cdirs/e3sm/www/xylar/analysis_testing/cori-haswell/mache_update_cori_base_url/main_py3.9

https://portal.nersc.gov/cfs/e3sm/xylar/analysis_testing/cori-haswell/mache_update_cori_base_url/main_py3.9

@xylar
Copy link
Collaborator

xylar commented Sep 1, 2022

@forsyth2, could you remove the trailing / for this file as well?
https://github.com/E3SM-Project/mache/blob/main/mache/machines/compy.cfg#L37

@forsyth2
Copy link
Contributor Author

forsyth2 commented Sep 1, 2022

@xylar Thanks for testing!

could you remove the trailing / for this file as well?

Sure, should I do that for the other settings too? I was running into the // issue with zppy as well, but since it worked I didn't mind so much. The issue is that all the settings don't match up on trailing /. For example:

https://github.com/E3SM-Project/mache/blob/main/mache/machines/cori-haswell.cfg:

base_path = /global/cfs/cdirs/e3sm/www

but https://github.com/E3SM-Project/mache/blob/main/mache/machines/compy.cfg:

base_path = /compyfs/www/

@xylar
Copy link
Collaborator

xylar commented Sep 1, 2022

@forsyth2, yes, please remove trailing / generally when you see them. Thank you!

@xylar
Copy link
Collaborator

xylar commented Sep 1, 2022

I probably won't re-test MPAS-Analysis after those changes because it's a lot of trouble to test on each machine. We'll just hope for the best...

@forsyth2 forsyth2 force-pushed the update-cori-base-url branch from 0f91bfe to cd06db2 Compare September 1, 2022 16:14
@forsyth2
Copy link
Contributor Author

forsyth2 commented Sep 1, 2022

please remove trailing / generally when you see them. Thank you!

@xylar I just pushed an amended commit. If that looks good to you, then it's ready to merge. Thanks!

Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks @forsyth2!

@xylar xylar merged commit 9dbdf21 into E3SM-Project:main Sep 1, 2022
@xylar
Copy link
Collaborator

xylar commented Sep 1, 2022

@forsyth2 and @chengzhuzhang, are you keen to have this functionality soon? I could do a 1.6.0 release if need be but would otherwise wait for other changes that might come soon (e.g. #48).

@chengzhuzhang
Copy link

Hey @xylar thanks for checking. We can wait, no need to have a separate release for this functionality...

@forsyth2 forsyth2 deleted the update-cori-base-url branch September 1, 2022 21:25
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.

Update Cori paths
3 participants