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 FourSeason run scripts with version information. #52

Merged
merged 8 commits into from
Nov 26, 2024

Conversation

AaronDonahue
Copy link
Collaborator

No description provided.

@forsyth2
Copy link
Collaborator

Thanks @AaronDonahue!

I've built the docs; the relevant changes are displayed at https://portal.nersc.gov/cfs/e3sm/forsyth/data_docs_52/html/SCREAMv0/DYAMOND2/simulation_data/index.html#original-runs-scripts and https://portal.nersc.gov/cfs/e3sm/forsyth/data_docs_52/html/SCREAMv1/FourSeasons/simulation_data/index.html#original-runs-scripts.

@PeterCaldwell @crterai Please review how the changes actually look in the built docs linked (as opposed to reviewing the pull request).

I'm personally not a fan of how the rst formatting makes it look like that section of the page is a whole separate page (on the sidebar table-of-contents), but I'm not sure how to fix that. Maybe a subheading instead?

@crterai
Copy link
Collaborator

crterai commented Nov 15, 2024

Thanks, @forsyth2. I agree that it's weird that the simulation data table seems to exist under the Original Runs Scripts heading rather than under Simulation Data. If that can be corrected with making Original Runs Scripts a sub-heading that'd be great. Also, I just noticed that it may be good to reword Original Runs Scripts to Original Run Scripts.

@AaronDonahue
Copy link
Collaborator Author

Also, I just noticed that it may be good to reword Original Runs Scripts to Original Run Scripts.

Thats a good suggestion Chris. I'll go ahead and make that change.

@AaronDonahue
Copy link
Collaborator Author

@forsyth2 , do you know what would happen if we moved

.. toctree::
   :maxdepth: 2
   :caption: Contents:

   simulation_table

to before the Original Run Scripts section? Would that move the link up?

Alternatively, we could just put Original Run Scripts at the top or we could change the Caption to read as Simulation Data rather than Contents.

Any thoughts?

@forsyth2
Copy link
Collaborator

https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#sections -- maybe try replacing the *** lines with =. I can try to play around with formatting this afternoon and see if something looks good.

@forsyth2
Copy link
Collaborator

No matter what I try, I can't seem to get the formatting right. Anything that makes "Original Run Scripts" a subsection/heading/part/etc. forces the sidebar to show the subsections rather than the child pages. I tried ***, ===, --- 1) below & above and 2) just below. On the sidebar (not the page itself), (1) puts the table under "Original Run Scripts" and keeps "Original Run Scripts" at the same indentation as "Simulation Data", whereas (2) puts "Original Run Scripts" indented under "Simulation Data" and doesn't show the table at all.

I also tried to just make it bold, but then it doesn't stand out at all amongst the rest of the bold text. .. rubric:: as suggested in https://stackoverflow.com/questions/7278253/using-sphinx-how-can-i-remove-the-title-appearing-in-the-side-bars-table-of-co has the same effect as making it bold.

Changing the caption from "Contents" to "Simulation Data" just changes how it looks on the web page; it doesn't affect the sidebar.

@forsyth2
Copy link
Collaborator

@AaronDonahue I think this is the best we can do:

Screenshot 2024-11-15 at 1 05 50 PM

What do you think (of the sidebar, and of the page layout)? The full link is https://portal.nersc.gov/cfs/e3sm/forsyth/data_docs_52/html/SCREAMv0/DYAMOND2/simulation_data/index.html

If that works, we just need @PeterCaldwell's approval to merge.

@AaronDonahue
Copy link
Collaborator Author

@forsyth2 , this looks good to me. Thank you for taking the time to reformat the page.

@PeterCaldwell
Copy link

Thanks all. It looks much better. It drives me batty that

If you have an account on NERSC, you can retrieve the data locally or remotely using Globus.
To download simulation data locally on a NERSC machine, refer to NERSC’s documentation here.
are in separate paragraphs since otherwise the text would split instructions cleanly into if/else conditions.

I'd also love it if that first block of text fell under a subtitle of "Instructions" because as is, the reader isn't really sure what to expect from that text.

But I can live without either of these changes.

@forsyth2
Copy link
Collaborator

@PeterCaldwell My latest commit f46ff94 addresses your comments. The changes can be seen at:

@AaronDonahue is there a particular reason why the instructions section differs so much between these 2 pages? I should also note for the <HPSS path below> on the second page, there is in fact no HPSS path below.

@AaronDonahue
Copy link
Collaborator Author

@forsyth2 , thanks for the fix and good catch on the HPSS. I believe that the SCREAMv0 page is a copy/paste from the E3SMv2 page which explains why they are different. I rewrote the SCREAMv1 page because I was unsure about using zstash for the FourSeasons data.

@crterai can you take a look at the 2nd link and advise on what to put here? Is there an HPSS that is viable for the DYAMOND1 runs?

@forsyth2
Copy link
Collaborator

I believe that the SCREAMv0 page is a copy/paste from the E3SMv2 page which explains why they are different.

@AaronDonahue This is a bit confusing. The v2 page actually more closely matches the SCREAM v1/Four Seasons page page than the SCREAM v0/DYAMOND2 page.

I was unsure about using zstash for the FourSeasons data.

zstash should be able to get anything off HPSS. Is there a specific concern about the FourSeasons data?

@chengzhuzhang
Copy link
Collaborator

zstash should be able to get anything off HPSS.

@forsyth2 looking into the file path, I don't think the SCREAM simulations are zstash archived. Files are archived as native model output, I think zstash won't apply in this case.

@forsyth2
Copy link
Collaborator

I don't think the SCREAM simulations are zstash archive

@chengzhuzhang oh I see, thanks.

@chengzhuzhang
Copy link
Collaborator

I don't think the SCREAM simulations are zstash archive

@chengzhuzhang oh I see, thanks.

No problem, we should revisit the pain points utilizing zstash for large files as those output from SCREAM and plan to accommodate.

@crterai
Copy link
Collaborator

crterai commented Nov 21, 2024

Hi @AaronDonahue and @forsyth2, So I looked at the v1 data link and I don't think zstash was used to archive either sets of simulations (v1 or v0). The file size of the scream output is so big so tar-ing up the data doesn't lead to better storage/ retrieval performance. So I think the v1 page can be written so that it removes the zstash reference and looks like the SCREAMv0 page. What do you think?

@crterai
Copy link
Collaborator

crterai commented Nov 21, 2024

@AaronDonahue @forsyth2 , I've updated the text so that it removes the reference to using zstash.

@forsyth2 forsyth2 force-pushed the aarondonahue/update_4season_scripts branch from 3723e22 to f719ae6 Compare November 22, 2024 19:02
@forsyth2
Copy link
Collaborator

Thanks @crterai I built the docs again, so these links are now updated:

@AaronDonahue @PeterCaldwell Let me know if you want to take another look at these pages. They look good to me. We can merge today.

@forsyth2
Copy link
Collaborator

@crterai approved, so I will merge. Thanks all!

@forsyth2 forsyth2 merged commit 6720d7b into main Nov 26, 2024
1 check passed
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.

5 participants