Skip to content

Adds in coastal flooding cases (Hurricane Sandy) #520

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
pwolfram merged 4 commits into from
Apr 30, 2020
Merged

Adds in coastal flooding cases (Hurricane Sandy) #520

pwolfram merged 4 commits into from
Apr 30, 2020

Conversation

@pwolfram
Copy link
Contributor

@pwolfram pwolfram commented Apr 14, 2020 •
edited
Loading

Enables land inundation for 4km, 2km, 1km Hurricane Sandy case

Includes the following more generalized features:

  • variable drag (*_variable_drag.xml)
  • use of wetting and drying for land inundation (*_wetting_drying_template.xml)
  • use of an AM to compute max water levels (maxwaterlevel_template.xml)

@pwolfram
Cleans up hurricane flooding init
f1ea992 
Removes unneeded vertical coordinate shift
@pwolfram pwolfram self-assigned this Apr 14, 2020
@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 14, 2020

@mark-petersen, only *.F change is to src/core_ocean/mode_init/mpas_ocn_init_hurricane.F to remove some unneeded lines, otherwise PR is COMPASS only.

@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 14, 2020

Note, this PR integrates multiple prior PRs:

@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 14, 2020

Note, case has not been explicitly tuned for accuracy against buoys or computational performance.

@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 14, 2020

Subset of #366

@pwolfram pwolfram mentioned this pull request Apr 14, 2020
Closed
@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 14, 2020 •
edited
Loading

Note, SAL is likely still broken and is turned off.

@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 14, 2020

Testing

8534720
8557380
8571421
8636580

8537121

Results are quite reasonable, even for a relatively low resolution of 4km.

pwolfram
pwolfram commented Apr 14, 2020
View reviewed changes
src/core_ocean/mode_init/mpas_ocn_init_hurricane.F Outdated
Copy link
Contributor Author

@pwolfram pwolfram Apr 14, 2020

Choose a reason for hiding this comment

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

@mark-petersen, if you can approve this merge into the MPAS-O fortran code, we can merge this in after @sbrus89 verifies it compiles and produces reasonable results for him.

Copy link
Contributor Author

@pwolfram pwolfram Apr 14, 2020

Choose a reason for hiding this comment

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

There are only about 6 lines and they remove changes needed to account for the fix you did ensuring we can run with layers above mean sea level.

sbrus89
sbrus89 reviewed Apr 14, 2020
View reviewed changes
Copy link

@sbrus89 sbrus89 left a comment

Choose a reason for hiding this comment

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

Everything looks good at this point; the main comment I have is that there needs to be a define_base_mesh.py file. We can also discuss whether the forward_RK4 step should be renamed forward.

@pwolfram pwolfram force-pushed the coastal/4km_wetting_drying branch from ff5bb2e to 990bc07 Compare April 14, 2020 23:20
@pwolfram
Adds flooding 4km Hurricane Sandy case
dc79a0c 
Includes the following more generalized features:
 * variable drag (*_variable_drag.xml)
 * use of wetting and drying for land inundation (*_wetting_drying_template.xml)
 * use of an AM to compute max water levels (maxwaterlevel_template.xml)
@pwolfram pwolfram force-pushed the coastal/4km_wetting_drying branch from 990bc07 to dc79a0c Compare April 14, 2020 23:23
@sbrus89
Copy link

sbrus89 commented Apr 16, 2020

@pwolfram, I ran the case and the results agree with yours. The only thing that seems off to me is the runtime. My job timed out after 4 hours with about an hour of simulation time left. It sounds like things are running a lot faster for you.

@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 16, 2020

Can you please retry with salloc -N 2 --time=04:00:00 --qos=interactive? This is how I've been testing that one.

@pwolfram pwolfram changed the title Adds in coastal flooding case to 4km Hurricane Sandy Adds in coastal flooding case to Hurricane Sandy Apr 16, 2020
@pwolfram
Adds flooding 2km Hurricane Sandy case
2bb2e67 
Builds off 2km Hurricane Sandy workflow
@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 16, 2020

2km case

No tuning past prior parameters for non-wetting and drying 2km case on commit 2bb2e67dfaf553db65bf2dbcaf41858f773f4ef9:

8534720
8537121
8557380
8636580
8510560

@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 16, 2020

@sbrus89, can you please also test the 2km case too? You'll probably need 10 nodes for 6 hrs. Mine ran for 4 hrs and timed out before it finished for the plots above.

@pwolfram pwolfram mentioned this pull request Apr 16, 2020
Open
@pwolfram pwolfram changed the title Adds in coastal flooding case to Hurricane Sandy Adds in coastal flooding cases (Hurricane Sandy and synthetic) Apr 16, 2020
@pwolfram pwolfram mentioned this pull request Apr 16, 2020
Merged
@pwolfram pwolfram force-pushed the coastal/4km_wetting_drying branch from c847270 to 315104a Compare April 18, 2020 19:50
@pwolfram pwolfram changed the title Adds in coastal flooding cases (Hurricane Sandy and synthetic) Adds in coastal flooding cases (Hurricane Sandy) Apr 18, 2020
mark-petersen
mark-petersen approved these changes Apr 20, 2020
View reviewed changes
Copy link
Contributor

@mark-petersen mark-petersen 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. If you guys are happy with this case, then go ahead and merge.

@pwolfram pwolfram force-pushed the coastal/4km_wetting_drying branch from 315104a to be0e74e Compare April 21, 2020 01:51
@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 21, 2020

@sbrus89, the 1km should be ready to test. It is on the queue for me (previous one was ok but time was too short). We should merge this week if possible.

@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 21, 2020

Note, this only considers Hurricane Sandy right now and no synthetic winds.

@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 22, 2020

1km testing

8534720
8537121
8557380
8573364
8631044
8632200

@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 22, 2020

@sbrus89, as far as I'm concerned we can merge this after you double check that it works for you. Results are improved overall, although obviously some tuning could occur, which is likely an issue of resolution as well as bottom drag.

@sbrus89
Copy link

sbrus89 commented Apr 22, 2020

Other than the minor change I suggested above, this is ready to go.

@sbrus89
Copy link

sbrus89 commented Apr 27, 2020

@pwolfram, should we get this merged this week?

@pwolfram pwolfram mentioned this pull request Apr 30, 2020
Merged
@pwolfram
Adds flooding 1km Hurricane Sandy case
d99009e 
Builds off 1km Hurricane Sandy workflow
@pwolfram pwolfram force-pushed the coastal/4km_wetting_drying branch from be0e74e to d99009e Compare April 30, 2020 15:26
@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 30, 2020

@sbrus89, can you please approve so I can merge? I think higher-res results should be in a separate PR, especially if a different bathymetry dataset is needed. Thanks!

@sbrus89 sbrus89 self-requested a review April 30, 2020 19:38
sbrus89
sbrus89 approved these changes Apr 30, 2020
View reviewed changes
Copy link

@sbrus89 sbrus89 left a comment

Choose a reason for hiding this comment

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

Thanks for all the great work on this, @pwolfram.

@pwolfram pwolfram merged commit 52dc6da into MPAS-Dev:ocean/coastal Apr 30, 2020
@pwolfram pwolfram deleted the coastal/4km_wetting_drying branch April 30, 2020 19:57
@pwolfram
Copy link
Contributor Author

pwolfram commented Apr 30, 2020

Thanks @sbrus89

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mark-petersen mark-petersen mark-petersen approved these changes

+1 more reviewer

@sbrus89 sbrus89 sbrus89 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@pwolfram pwolfram

Labels

coastal COMPASS feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pwolfram @sbrus89 @mark-petersen @matthewhoffman