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.

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

Clean up BarotropicQG module #147

Merged
merged 10 commits into from
Nov 27, 2020
Merged

Clean up BarotropicQG module #147

merged 10 commits into from
Nov 27, 2020

Conversation

navidcy
Copy link
Member

@navidcy navidcy commented Nov 25, 2020 •
edited
Loading

This PR removes the large-scale zonal flow U(t) functionality from the BarotropicQG module.

Closes #135 and opens room for #137.

@navidcy navidcy changed the title Clean up BarotropicQG module [WIP] Clean up BarotropicQG module Nov 25, 2020
@navidcy
Copy link
Member Author

navidcy commented Nov 25, 2020 •
edited
Loading

I'm wondering whether I should try to enhance the barotropic_acc.jl example to include the U(t) functionality + topographic form stress forcing. I'm not even sure that can be done relatively simply.

Otherwise, we should include an example of BarotropicQG module with non-trivial topography.

@glwagner, @BrodiePearson opinions?

@navidcy
Copy link
Member Author

navidcy commented Nov 25, 2020 •
edited
Loading

I added an example with decaying 2D turbulence (BarotropiqQG with β=0) over topography.

This is ready to be reviewed.

The new example in the docs can be previewed at:
https://fourierflows.github.io/GeophysicalFlowsDocumentation/previews/PR147/generated/barotropicqg_decaying_topography/

@navidcy navidcy requested review from glwagner and BrodiePearson and removed request for glwagner November 25, 2020 10:08
@navidcy navidcy changed the title [WIP] Clean up BarotropicQG module Clean up BarotropicQG module Nov 25, 2020
BrodiePearson
BrodiePearson reviewed Nov 27, 2020
View reviewed changes
Copy link
Collaborator

@BrodiePearson BrodiePearson 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 to me! Nice example and animation.

examples/barotropicqg_decaying_topography.jl Outdated Show resolved Hide resolved
examples/barotropicqg_decaying_topography.jl Outdated Show resolved Hide resolved
examples/barotropicqg_decaying_topography.jl Outdated Show resolved Hide resolved
examples/barotropicqg_decaying_topography.jl Outdated Show resolved Hide resolved
@navidcy navidcy removed the request for review from glwagner November 27, 2020 21:39
@navidcy navidcy merged commit dee0721 into master Nov 27, 2020
@navidcy navidcy deleted the ncc/remove-U-from-BarotropicQG branch November 27, 2020 22:37
@navidcy
Copy link
Member Author

navidcy commented Nov 27, 2020

@BrodiePearson I addressed all your remarks ;)

@navidcy navidcy removed the request for review from BrodiePearson November 27, 2020 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BrodiePearson BrodiePearson BrodiePearson left review comments

Assignees
No one assigned
Labels
🧹 cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove large-scale flow U(t) from BarotropicQG
2 participants
@navidcy @BrodiePearson