Skip to content

Remove reliance on ocean_mask.nc for tidal related files #79

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Aug 13, 2025
Merged

Remove reliance on ocean_mask.nc for tidal related files #79

merged 3 commits into from
Aug 13, 2025

Conversation

minghangli-uni
Copy link
Collaborator

closes #78

@minghangli-uni
Copy link
Collaborator Author

I'll generate the two files to confirm they are identical.

@minghangli-uni
Copy link
Collaborator Author 

$ nccmp -d bottom_roughness_topog_generated.nc /g/data/vk83/prerelease/configurations/inputs/access-om3/mom/tidal_external_files/global.25km/2025.05.15/bottom_roughness.nc

$ nccmp -d tideamp_topog_generated.nc /g/data/vk83/prerelease/configurations/inputs/access-om3/mom/tidal_external_files/global.25km/2025.05.15/tideamp.nc
DIFFER : NAMES OF RECORDS : time <>

The tideamp.nc files differ because the newly generated version includes an additional time dimension. #76

@minghangli-uni minghangli-uni self-assigned this Aug 11, 2025
@minghangli-uni
Copy link
Collaborator Author

@anton-seaice can you help review this pr?

@minghangli-uni minghangli-uni marked this pull request as ready for review August 11, 2025 04:10
anton-seaice
anton-seaice reviewed Aug 12, 2025
View reviewed changes
anton-seaice
anton-seaice reviewed Aug 12, 2025
View reviewed changes
Copy link
Contributor

@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.

Looks good - minor comments :)

@minghangli-uni
Copy link
Collaborator Author

@anton-seaice Could you please take another look?

anton-seaice
anton-seaice approved these changes Aug 13, 2025
View reviewed changes
Copy link
Contributor

@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.

Looks good. Thanks @minghangli-uni

The example output / test results are very useful when reviewing !

@minghangli-uni minghangli-uni merged commit 6029f2a into main Aug 13, 2025
4 checks passed
@minghangli-uni minghangli-uni deleted the 78-topog-tidal-related-files branch August 13, 2025 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@anton-seaice anton-seaice anton-seaice approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@minghangli-uni minghangli-uni

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove reliance on ocean_mask.nc for tidal related files

2 participants

@minghangli-uni @anton-seaice