-
Notifications
You must be signed in to change notification settings - Fork 12
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
Tests for Terminator Toy and SplitPrescribedTransport #465
Conversation
@@ -37,7 +37,7 @@ def run_wind_drag(dirname, implicit_formulation): | |||
eqn = CompressibleEulerEquations(domain, parameters) | |||
|
|||
# I/O | |||
output = OutputParameters(dirname=dirname+"/surface_fluxes", | |||
output = OutputParameters(dirname=dirname+"/wind_drag", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again Tim for doing this.
I think the terminator toy test looks sensible. I had a couple of suggestions to slightly simplify it. The other changes look good.
species2_name='X2'), BackwardEuler(domain))] | ||
|
||
# Set up a non-divergent, time-varying, velocity field | ||
def u_t(t): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the wind is zero, do we need a function prescribing the velocity? Or could we just set the initial velocity to zero?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find a way to get the prescribed_transporting_velocity to work without giving it a function.
# Make the mesh and domain | ||
R = 6371220. | ||
mesh = IcosahedralSphereMesh(radius=R, | ||
refinement_level=3, degree=2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this is just a test and the spatial discretisation is not important, could we use a coarser grid? e.g. refinement level 2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. Using a refinement level of 2 still successfully tests the ODEs moving the system towards the steady state, although I will need to halve the timestep size and slacken the tolerance on the test (error less than 0.4) to adjust.
X_field = stepper.fields("X") | ||
X2_field = stepper.fields("X2") | ||
|
||
# Assert that the physics scheme has sufficiently moved |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a sensible test to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As everything is passing I think this is now ready! Thanks Tim
This pull request follows up on PR#457 to add the following tests:
A test of the prescribed wind velocity feature of SplitPrescribedTransport. This is implemented as an additional parameterised option in the test_prescribed_transport.py file.
Using SplitPrescribedTransport within the test_precipitation physics test.