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 dam break test case #171

Closed

Conversation

caozd999
Copy link

@caozd999 caozd999 commented Mar 4, 2019

Adds dam break test case for comparison against experimental and ROMS results to ensure that MPAS-O flooding wave is accurate.

@caozd999 caozd999 changed the base branch from master to ocean/develop March 4, 2019 17:10
@caozd999 caozd999 changed the title Ocean/dam_break_new add dam break test case Mar 4, 2019
@caozd999 caozd999 closed this Mar 4, 2019
@pwolfram
Copy link
Contributor

pwolfram commented Mar 4, 2019

@caozd999, we do want to submit a PR and you should be able to reopen. Thanks for taking the initiative to start the PR.

How about cherry-picking 208bf289051d30d20554288a7c658f65648487b6 and d242282eea8cdd820317dbfd3ff1a5becd9d1a67 from https://github.com/pwolfram/MPAS-Model/commits/sbrus89/time-varying-wind-unsquashed_wetting onto the latest branch of ocean/develop and making sure it works?

@pwolfram pwolfram reopened this Mar 4, 2019
@xylar
Copy link
Collaborator

xylar commented Mar 4, 2019

@pwolfram, I talked with @caozd999 about this this morning. I agree, cherry-picking only a few necessary commits is the right way to do this. I'm happy to help with the process as needed.

@pwolfram
Copy link
Contributor

pwolfram commented Mar 5, 2019

@caozd999, does this make sense? Please go ahead and do this and once you confirm the code builds and runs against your test case I will review. Please feel free to include your data sources and comparison script in this PR too as there is not a commit for that. Please let me and @xylar as you have questions about how to go about doing this PR.

@caozd999
Copy link
Author

caozd999 commented Mar 5, 2019

@pwolfram @xylar Thanks. I will work on it today.

@caozd999
Copy link
Author

caozd999 commented Mar 6, 2019

@xylar and I worked on the cherry-pick d242282, and we found merge conflicts in testing_and_setup/compass/ocean/jigsaw_to_MPAS/inject_bathymetry.py which we don't know how to fix. Could you do the rebase on the ocean/develop on that commit please?

@xylar
Copy link
Collaborator

xylar commented Mar 6, 2019

@pwolfram, I think the problem is that @mark-petersen has made changes to that file on ocean/develop and you made others on the wetting-and-drying branch (among the 198 files you altered there! :-)), and it wasn't at all obvious how to resolve the conflicts. But it was a really good demo of git cherry-pick for @caozd999 to learn from!

@xylar
Copy link
Collaborator

xylar commented Mar 26, 2019

@pwolfram, I believe @caozd999 could use your help with this one. Just wanted to ping you both to make sure this doesn't get dropped entirely.

@caozd999 caozd999 force-pushed the ocean/dam_break_new branch from 9f39681 to 5f5f4f4 Compare April 1, 2019 18:23
@pwolfram
Copy link
Contributor

pwolfram commented Apr 4, 2019

@xylar, the issue is this requires wetting and drying. I'll sort this out and we should merge this case under that umbrella.

@xylar
Copy link
Collaborator

xylar commented Apr 4, 2019

Should this PR be closed since my understanding is that #54 brings in this functionality?

@pwolfram
Copy link
Contributor

pwolfram commented Apr 5, 2019

Yes

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

Successfully merging this pull request may close these issues.

4 participants