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

How to build an ACCESS model page #821

Merged
merged 2 commits into from
Oct 13, 2024
Merged

Conversation

atteggiani
Copy link
Contributor

@atteggiani atteggiani commented Oct 4, 2024

Fixes #743.

  • Added 'How to build an ACCESS model' page
  • Linked the newly-created page to the 'How to run ...' pages
  • Other minor fixes:
    • Added link to ACCESS-ESM1.5-configs to ACCESS-ESM1.5 page
    • Added ids to tabs buttons in ACCESS-OM page (needed for tabs functionality)
    • Updated terminal-animations.js to version 3.0

@atteggiani atteggiani self-assigned this Oct 4, 2024
@atteggiani atteggiani requested review from harshula, aidanheerdegen and a team October 4, 2024 06:07
Copy link

github-actions bot commented Oct 4, 2024

PR preview
⚠️ There was an error in the pr-preview deployment. For more information, please check the Actions tab.
2024-10-14 10:24 AEDT

@atteggiani atteggiani requested a review from heidinett October 4, 2024 06:17
Copy link
Contributor

@heidinett heidinett left a comment

Choose a reason for hiding this comment

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

Looks good @atteggiani! I have made a few suggestions for edits to the text.

@atteggiani
Copy link
Contributor Author

Looks good @atteggiani! I have made a few suggestions for edits to the text.

Thank you for the reivew @heidinett!
All your comments have been addressed.

@atteggiani atteggiani requested review from heidinett and a team October 8, 2024 23:14
Copy link
Contributor

@heidinett heidinett left a comment

Choose a reason for hiding this comment

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

Thanks @atteggiani. Looks good!

Copy link

@anton-seaice anton-seaice left a comment

Choose a reason for hiding this comment

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

Thanks @harshula and @atteggiani - this will be useful!

docs/models/run-a-model/build_a_model.md Show resolved Hide resolved
docs/models/run-a-model/build_a_model.md Show resolved Hide resolved
docs/models/run-a-model/build_a_model.md Show resolved Hide resolved
@atteggiani
Copy link
Contributor Author

Addressed @harshula's comments (plus some additional ones discussed via chat) in 13506e3c5e8c940d2ffd71ea48a334ce4f1046f4

Copy link
Member

@aidanheerdegen aidanheerdegen left a comment

Choose a reason for hiding this comment

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

I've read through and they are great. Clear with enough context to make them easily understood. Thanks everyone for the work to get to this point.

My only concern is that we're using ESM1.5 as the example, and so building the UM which has some licensing conditions attached. We're not giving users any indication that they should perhaps be more careful, restrict access or otherwise take steps to avoid violating license conditions.

I don't suppose it is greatly different to existing practice where users checkout source code from the svn mirror, but these instructions are quite prominent.

@atteggiani
Copy link
Contributor Author

I've read through and they are great. Clear with enough context to make them easily understood. Thanks everyone for the work to get to this point.

My only concern is that we're using ESM1.5 as the example, and so building the UM which has some licensing conditions attached. We're not giving users any indication that they should perhaps be more careful, restrict access or otherwise take steps to avoid violating license conditions.

I don't suppose it is greatly different to existing practice where users checkout source code from the svn mirror, but these instructions are quite prominent.

I think that raises a valid point.
Do you think we can add a general warning at the beginning flagging the licensing reauirements if they are building models that include the UM?

@aidanheerdegen
Copy link
Member

Do you think we can add a general warning at the beginning flagging the licensing reauirements if they are building models that include the UM?

Yes that sounds like a good idea. It doesn't have to happen in this PR, but for the future it would make sense to have a separate page about UM licensing, how it works, who is eligible and what their responsibilities are. Then we could just refer to it.

Copy link
Contributor

@harshula harshula 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, well done! Thanks!

Applied Heidi's comments

Addressed Harshula's comments + minor other fixes:
- Specified Spack **managed** environments when necessary
- Fix output bad formatting in virtual terminals
- Changed reference to $(prefix) to point out $spack instead
- Changed part for the output formatting to refer to the specific Spack instance installed throught the setup
- Changed a 'data="input"' line to 'data="output"' for the virtual terminal related to "spack find mom5"

Addressed Harshula and Anton's comments

Added Harshula's suggestions.
@atteggiani atteggiani merged commit a610064 into development Oct 13, 2024
2 of 4 checks passed
@atteggiani atteggiani deleted the davide/spack-build branch October 13, 2024 23:23
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.

Add a document on how to use Spack develop
5 participants