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

Cleanup PR #1 #153

Merged
merged 4 commits into from
Nov 21, 2024
Merged

Cleanup PR #1 #153

merged 4 commits into from
Nov 21, 2024

Conversation

TeaganKing
Copy link
Collaborator

@TeaganKing TeaganKing commented Nov 21, 2024

Description of changes:

Address #151 by moving nblibrary out of examples and cleaning up the names of output directories. This PR also updates documentation to point to key_metrics example.

@TeaganKing TeaganKing self-assigned this Nov 21, 2024
@TeaganKing
Copy link
Collaborator Author

Note: In addition to the items mentioned in the related issue, I am updating our documentation to use the key_metrics example instead of the coupled_model example.

@TeaganKing TeaganKing linked an issue Nov 21, 2024 that may be closed by this pull request
@TeaganKing
Copy link
Collaborator Author

TeaganKing commented Nov 21, 2024

For the key_metrics example, the rof notebooks seem to be failing for some reason, but the other notebooks work as expected with these changes and cupid-run and cupid-build both complete successfully with output looking as expected in the resulting website.

The ADF link in external_diag_packages seems to be working but the actual in-line plots are not showing up, so this needs to be further tested. Is this possibly related to the change @mnlevy1981 made in #154?

@mnlevy1981
Copy link
Collaborator

mnlevy1981 commented Nov 21, 2024

The ADF link in external_diag_packages seems to be working but the actual in-line plots are not showing up, so this needs to be further tested. Is this possibly related to the change @mnlevy1981 made in #154?

In the external_diag_packages directory, you'll need to modifyconfig.yml so adf_root points to the right place:

- adf_root: ../../external_diag_packages/ADF/
+ adf_root: ../../externals/external_diag_packages/ADF/

adf_root is relative to where the notebook is run, which used to be ${CUPID_ROOT}/externals/nblibrary/atm but is now ${CUPID_ROOT}/nblibrary/atm

@mnlevy1981
Copy link
Collaborator

The ADF link in external_diag_packages seems to be working but the actual in-line plots are not showing up, so this needs to be further tested. Is this possibly related to the change @mnlevy1981 made in #154?

In the external_diag_packages directory, you'll need to modifyconfig.yml so adf_root points to the right place:

- adf_root: ../../external_diag_packages/ADF/
+ adf_root: ../../externals/external_diag_packages/ADF/

adf_root is relative to where the notebook is run, which used to be ${CUPID_ROOT}/externals/nblibrary/atm but is now ${CUPID_ROOT}/nblibrary/atm

Actually, you want

- adf_root: ../../external_diag_packages/ADF/
+ adf_root: ../../externals/external_diag_packages/ADF_output/

(In #141 I changed the location of the ADF output in build.py, and I guess I never deleted externals/external_diag_packages/ADF so my testing didn't catch the change in adf_root until I started working on #154)

@TeaganKing
Copy link
Collaborator Author

I fixed the ADF path by setting the adf_root to ../../examples/external_diag_packages/ADF_output/.

I'll work on the rof notebooks outside this PR.

This is ready for review.

@TeaganKing TeaganKing marked this pull request as ready for review November 21, 2024 21:26
Copy link
Collaborator

@mnlevy1981 mnlevy1981 left a comment

Choose a reason for hiding this comment

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

This looks great! I'm baffled by the ROF issue, especially because it looks like the notebook actually ran fine despite throwing a bunch of cryptic errors

@mnlevy1981
Copy link
Collaborator

FYI, it looks like we never added the runoff notebooks to the Jupyter Book... config.yml will need

    - caption: River Runoff
      chapters:
        - file: rof/global_discharge_gauge_compare_obs
        - file: rof/global_discharge_ocean_compare_obs

Where the rest of the chapters are defined

@TeaganKing TeaganKing merged commit 71c4ef8 into NCAR:main Nov 21, 2024
2 checks passed
@TeaganKing
Copy link
Collaborator Author

FYI, it looks like we never added the runoff notebooks to the Jupyter Book... config.yml will need

    - caption: River Runoff
      chapters:
        - file: rof/global_discharge_gauge_compare_obs
        - file: rof/global_discharge_ocean_compare_obs

Where the rest of the chapters are defined

Ack, thanks for catching that! I remember looking at it so must have updated this in my own version somewhere... I'll follow up on that outside this PR.

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.

move nblibrary out of examples & clean up names of output dirs
2 participants