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

Add topo gen workflow #1

Merged
merged 15 commits into from
Dec 9, 2024
Merged

Add topo gen workflow #1

merged 15 commits into from
Dec 9, 2024

Conversation

ezhilsabareesh8
Copy link
Collaborator

This PR adds the topography generation workflow from the GEBCO 2024 dataset.

  • doamin-tools is added as submodule and it will be build by build.sh
  • Two hand edits have been made to include the black sea in the topography as discussed here
  • Finalise.sh adds the git hash to the file history

Example Netcdf file history of topog.nc:

:history = "Tue Nov 20 16:59:49 2024 +1100: topogtools min_max_depth -i topog_new_fillfraction_edited_deseas.nc -o topog_new_fillfraction_edited_deseas_mindepth.nc
 --level 4 --vgrid ocean_vgrid.nc --vgrid_type mom6\nTue 
Nov 20 16:59:49 2024 +1100: topogtools deseas -i topog_new_fillfraction_edited.nc -o topog_new_fillfraction_edited_deseas.nc --grid_type C\nTue 
Nov 20 16:59:45 2024 +1100: topogtools fill_fraction -i topog_new_min_dy.nc -o topog_new_fillfraction.nc --fraction 0.5\nTue 
Nov 20 16:59:45 2024 +1100: topogtools min_dy -i topog_new.nc -o topog_new_min_dy.nc --cutoff 6000 --hgrid ocean_hgrid.nc\nMon Nov 19 11:57:43 2024 +1100: topogtools gen_topo -i GEBCO_2024.nc -o topog_new.nc --hgrid ocean_hgrid.nc --tripolar --longitude-offset -100 | 
Wed Nov 20 16:59:48 2024 ./topogtools/editTopo.py --overwrite --nogui --apply edit_topog_new_fillfraction.txt --output topog_new_fillfraction_edited.nc topog_new_fillfraction.nc made 2 changes (i, j, old, new): (1235, 737, 0.0, 65.28469848632812), (1227, 734, 0.0, 56.118499755859375) | 
Created on Wed Nov 20 17:14:54 AEDT 2024 using https://github.com/ACCESS-NRI/make_OM3_025deg_topo/tree/3093206 and based on GEBCO_2024 topography" ;
}

@anton-seaice
Copy link
Collaborator

Does finalise.sh need to be a seperate script? Can it be part of gen_topo.sh ?

In other places, we normally expect that the script is run on code that is already committed, rather than commit it during the script.

e.g. https://github.com/COSIMA/om3-scripts/blob/752c41d5a4df73b8489a0a428cbfc63b8607ec86/scripts_common.py#L119

gen_topo.sh Outdated Show resolved Hide resolved
gen_topo.sh Outdated Show resolved Hide resolved
gen_topo.sh Show resolved Hide resolved
gen_topo.sh Show resolved Hide resolved
build.sh Show resolved Hide resolved
gen_topo.sh Outdated Show resolved Hide resolved
gen_topo.sh Show resolved Hide resolved
gen_topo.sh Outdated Show resolved Hide resolved
gen_topo.sh Outdated Show resolved Hide resolved
gen_topo.sh Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@micaeljtoliveira
Copy link
Member

A simple way to simplify this a little bit would be to add editTopo.py to domain-tools. That way you would only have two git submodules instead of three. If that sounds like a good idea, let me know and I can take care of it.

@ezhilsabareesh8
Copy link
Collaborator Author

A simple way to simplify this a little bit would be to add editTopo.py to domain-tools. That way you would only have two git submodules instead of three. If that sounds like a good idea, let me know and I can take care of it.

Thanks @micaeljtoliveira, It sounds good to me.

@ezhilsabareesh8
Copy link
Collaborator Author

Does finalise.sh need to be a seperate script? Can it be part of gen_topo.sh ?

I think the topography files needs to be checked before pushed to github, as mentioned here in the old topography generation workflow here

@ezhilsabareesh8
Copy link
Collaborator Author

@anton-seaice Addressed all the comments in this commit 6977462

gen_topo.sh Outdated Show resolved Hide resolved
finalise.sh Show resolved Hide resolved
gen_topo.sh Outdated Show resolved Hide resolved
gen_topo.sh Outdated Show resolved Hide resolved
gen_topo.sh Outdated Show resolved Hide resolved
gen_topo.sh Outdated Show resolved Hide resolved
gen_topo.sh Outdated Show resolved Hide resolved
edit_topog_new_fillfraction.txt Outdated Show resolved Hide resolved
@anton-seaice
Copy link
Collaborator

Does finalise.sh need to be a seperate script? Can it be part of gen_topo.sh ?

I think the topography files needs to be checked before pushed to github, as mentioned here in the old topography generation workflow here

We can't rely on people remembering to do this manual step after generating the topo, it needs to be automated

The script should give a warning / fail at the start if its not committed and pushed to github

@anton-seaice
Copy link
Collaborator

If this is generalised to make the topography at all resolution, we can rename the repository too.

@ezhilsabareesh8
Copy link
Collaborator Author

We can't rely on people remembering to do this manual step after generating the topo, it needs to be automated

Automation isn’t feasible here since the workflow involves ongoing manual edits until the topography is finalized. Even with automated checks for certain parameters, the user must verify that the topography meets the desired level of satisfication.

@ezhilsabareesh8
Copy link
Collaborator Author

@anton-seaice All the comments are addressed, as we discussed in the meeting yesterday. Please let me know if there's any additional changes requires.

@anton-seaice
Copy link
Collaborator

Hi @ezhilsabareesh8 - I made some minor tidy ups.

When i run this it fails at this step:

Opened "topog_new_fillfraction.nc". Will write to "topog_new_fillfraction_edited.nc" when window is closed.
Range of input depths: min= 0.0 max= 9700.796
Error: There was a problem applying edits from "edit_025deg_topog_new_fillfraction.txt".

?

@ezhilsabareesh8
Copy link
Collaborator Author

"edit_topog_new_fillfraction.txt" needs to be renamed to match gen_topo.sh. I did it in this commit 41934bd

@anton-seaice
Copy link
Collaborator

A simple way to simplify this a little bit would be to add editTopo.py to domain-tools. That way you would only have two git submodules instead of three. If that sounds like a good idea, let me know and I can take care of it.

@micaeljtoliveira - are you planning to do this ?

@micaeljtoliveira
Copy link
Member

@anton-seaice Yes, I'll do it today.

@anton-seaice
Copy link
Collaborator

@ezhilsabareesh8 - I added some more information about the input files used to the metadata, and improved the comment for the git commit. I think the only thing outstanding is the move of editTopo.py

@ezhilsabareesh8
Copy link
Collaborator Author

Thanks @anton-seaice for tidying up this PR and enhancing the metadata.

@micaeljtoliveira
Copy link
Member

editTopo.py is now part of domain-tools, which has been renamed to bathymetry-tools. Changing a git submodule can sometimes be quite convoluted, so I can that care of that if no one objects and if there are no other changes needed for this PR to be accepted.

@micaeljtoliveira
Copy link
Member

@ezhilsabareesh8 @anton-seaice I've now changed the scripts to use the latest version of the bathymetry tools (aka domain-tools). Could one of you check that it's working correctly?

@ezhilsabareesh8
Copy link
Collaborator Author

Thanks @micaeljtoliveira, I checked the changes, It is working fine.

Copy link
Collaborator

@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 @ezhilsabareesh8 - looks good

@ezhilsabareesh8 ezhilsabareesh8 merged commit 283f125 into main Dec 9, 2024
@ezhilsabareesh8 ezhilsabareesh8 mentioned this pull request Dec 9, 2024
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.

3 participants