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

Deep convection example and golden master test should switch to BCs. #220

Closed
ali-ramadhan opened this issue May 14, 2019 · 11 comments
Closed
Assignees
Labels
testing 🧪 Tests get priority in case of emergency evacuation

Comments

@ali-ramadhan
Copy link
Member

Instead of a forcing function it should use a Flux boundary condition specified with an array.

@ali-ramadhan ali-ramadhan self-assigned this May 14, 2019
@ali-ramadhan ali-ramadhan added the testing 🧪 Tests get priority in case of emergency evacuation label May 14, 2019
@glwagner
Copy link
Member

Could possibly nuke the deep convention test (the example was already nuked) and close this.

@christophernhill
Copy link
Member

christophernhill commented May 29, 2019 via email

@glwagner
Copy link
Member

Aha! The deep convection tests are currently marked as 'broken' and therefore ignored.

In the meantime, we have a new Rayleigh-Benard regression test that tests most of the same features that were tested by deep convention (and a few additional ones, as well).

We probably can't afford too many regression tests (since they are somewhat expensive), and we will want to add ones for new operators / LES closures / etc, so its probably the right decision in the long run to 'nuke' the deep convection test.

@christophernhill
Copy link
Member

christophernhill commented May 29, 2019 via email

@ali-ramadhan
Copy link
Member Author

Yeah I wanted to make deep convection one of the examples we maintain so it can also be a regression test to make sure the example always works as expected. It's not that expensive, but if we can make it faster that would be great. I suppose not using NetCDF would be a start.

@glwagner
Copy link
Member

Ok, ok. If it's rotating, uses Flux bcs at the top, Gradient bcs at the bottom, and is non-trivially stratified in both salinity and temperature, it'll test a good number of features in addition to the Rayleigh-Benard test.

@ali-ramadhan
Copy link
Member Author

Does linear stratification count as non-trivially stratified?

But yeah, adding a bit of salinity in there would be good for the test, maybe not so much for the example.

@johncmarshall54
Copy link

johncmarshall54 commented May 29, 2019 via email

@glwagner
Copy link
Member

Aha, yes! By non-trivial I meant non-zero, I suppose. Just something to get some dynamics going.

The Rayleigh-Benard test uses salinity as a passive tracer (and forces it), so we actually capture passive tracer advection-diffusion with that regression test. By adding salinity-stratification, we ensure the modeled effects of salinity on buoyancy also do not regress.

@glwagner
Copy link
Member

Moot since deep convection regression was nuked?

@ali-ramadhan
Copy link
Member Author

Ah this was a very minor issue, technically resolved in PR #334, although relevant again as comparing deep convection with MITgcm is part of the verification experiments we agreed upon (see #346).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing 🧪 Tests get priority in case of emergency evacuation
Projects
None yet
Development

No branches or pull requests

4 participants