Conversation
9869df2 to
8461420
Compare
|
cscs-ci run default |
1 similar comment
|
cscs-ci run default |
|
cscs-ci run distributed |
|
cscs-ci run default |
|
cscs-ci run distributed |
|
cscs-ci run default |
1 similar comment
|
cscs-ci run default |
|
cscs-ci run default |
|
cscs-ci run default |
| # TODO(halungge): Decouple the vertical from horizontal grid. | ||
| vertical_size: int | ||
| limited_area: bool = True | ||
| iau_init: bool = False |
There was a problem hiding this comment.
I think iau_init should not be a property of grid because it is only mostly related to how we initialize the numerical setup and read the data, nothing to do with the underlying grid, Although our python driver is still far from that stage, the best place to add this boolean switch is probably DriverConfig in standalone_driver. Could you either add a TODO here saying that this will be moved to DriverConfig or just simply move there if you have time?
There was a problem hiding this comment.
So iau_init should be in the driver config, yes. We unfortunately do not have such a driver config object being passed or being constructed when we go from the blue line. We only have grid_init, solve_nh_init and diffusion_init. Since iau_init is a global parameter in ICON (also affects OCEAN), I put it into grid_init. Should we do a global_init for the blue line?
| at_first_substep=self._is_first_substep(dyn_substep), | ||
| at_last_substep=self._is_last_substep(dyn_substep), | ||
| is_iau_active=False, | ||
| iau_wgt_dyn=0.0, |
| lprep_adv=lprep_adv, | ||
| at_first_substep=(substep_init == 1), | ||
| at_last_substep=(substep_init == ndyn_substeps), | ||
| is_iau_active=False, |
There was a problem hiding this comment.
I added is_iau_active and iau_wgt_dyn to the list of fixtures.
There was a problem hiding this comment.
You now can use the fixture as you did in test_solve_nonhydro.py
| is_iau_active=False, | |
| is_iau_active=is_iau_active, | |
| iau_wgt_dyn=iau_wgt_dyn, |
|
cscs-ci run default |
1 similar comment
|
cscs-ci run default |
|
cscs-ci run distributed |
|
cscs-ci run default |
|
cscs-ci run distributed |
|
cscs-ci run default |
|
cscs-ci run distributed |
2 similar comments
|
cscs-ci run distributed |
|
cscs-ci run distributed |
|
cscs-ci run default |
1 similar comment
|
cscs-ci run default |
OngChia
left a comment
There was a problem hiding this comment.
Thanks! I think it is overall good. I left two comments about the usage of fixtures. Otherwise good!
| ) | ||
|
|
||
| from .. import utils | ||
| from ..fixtures import * # noqa: F403 |
There was a problem hiding this comment.
You can keep this and add iau_wgt_dyn and icon_grid in model/atmosphere/dycore/tests/dycore/fixtures.py instead of from directly importing icon4py.model.testing.fixtures as you do on Line 30.
| lprep_adv=lprep_adv, | ||
| at_first_substep=(substep_init == 1), | ||
| at_last_substep=(substep_init == ndyn_substeps), | ||
| is_iau_active=False, |
There was a problem hiding this comment.
You now can use the fixture as you did in test_solve_nonhydro.py
| is_iau_active=False, | |
| is_iau_active=is_iau_active, | |
| iau_wgt_dyn=iau_wgt_dyn, |
| lowest_layer_thickness: ta.wpfloat, | ||
| model_top_height: ta.wpfloat, | ||
| stretch_factor: ta.wpfloat, | ||
| damping_height: ta.wpfloat, |
There was a problem hiding this comment.
Here you add is_iau_active and iau_wgt_dyn
| is_iau_active: bool, | |
| iau_wgt_dyn: ta.wpfloat, |
|
cscs-ci run default |
jcanton
left a comment
There was a problem hiding this comment.
more fixtures to cleanup, yay :-)
|
Mandatory Tests Please make sure you run these tests via comment before you merge!
Optional Tests To run benchmarks you can use:
To run tests and benchmarks with the DaCe backend you can use:
To run test levels ignored by the default test suite (mostly simple datatest for static fields computations) you can use:
For more detailed information please look at CI in the EXCLAIM universe. |
|
cscs-ci run default |
No description provided.