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

Fix SST/SSS WOA13 comparison and equatorial thermal/zonal velocity examples #283

Merged
merged 7 commits into from
Oct 1, 2023

Conversation

fabiobdias
Copy link
Collaborator

@fabiobdias fabiobdias commented Sep 4, 2023

Closes #269 and closes #268

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@fabiobdias fabiobdias changed the title Fix SST/SSS WOA13 comparison example Fix SST/SSS WOA13 comparison and equatorial thermal/zonal velocity examples Sep 4, 2023
@navidcy
Copy link
Collaborator

navidcy commented Sep 4, 2023

@hrsdawson can you review this? as part of the review can you ensure the two notebooks run if you do "restart kernel and run all cells"?

Copy link
Collaborator

@navidcy navidcy left a comment

Choose a reason for hiding this comment

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

@fabiobdias please uncomment also the other notebook from the test file

Copy link
Collaborator

@navidcy navidcy left a comment

Choose a reason for hiding this comment

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

lgtm except what are all these LOOOONG lists of warnings about date time?

@navidcy
Copy link
Collaborator

navidcy commented Sep 6, 2023

@micaeljtoliveira do you know why these looooong warnings come up?

Copy link
Collaborator

@rmholmes rmholmes left a comment

Choose a reason for hiding this comment

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

I checked these out and ran them and they all look good to me. Figures look consistent with the results I expect. Thanks @fabiobdias

My only comments would be:

  • The equatorial longitude-depth slices are in both the Compare_SST_SSS... and Equatorial_thermal... files, they could probably just be in the Equatorial_thermal... file.
  • The WOA database should probably be put somewhere public so that it does not have to be regenerated by every user (although it doesn't take long, so probably don't bother).

I think these are good example scripts that could/should be picked up for ACCESS-OM3 and/or the NRI for standard model validation checks.

@micaeljtoliveira
Copy link
Contributor

@micaeljtoliveira do you know why these looooong warnings come up?

Sorry, no idea 😞

@navidcy navidcy merged commit 990316f into main Oct 1, 2023
2 checks passed
@navidcy navidcy deleted the sst-sss_obs_comparison branch October 1, 2023 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants