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

Fix solve nonhydro divdamp fac o2 #313

Merged
merged 29 commits into from
Nov 28, 2023
Merged

Fix solve nonhydro divdamp fac o2 #313

merged 29 commits into from
Nov 28, 2023

Conversation

halungge
Copy link
Contributor

@halungge halungge commented Nov 15, 2023 •
edited
Loading

divdamp_fac_o2 changes externally SolveNonHydro. (In ICON: see mo_nh_stepping.f90 SUBROUTINE update_spinup_damping. )

  • (fix) remove divdamp_fac_o2 from NhConstants class and pass it as parameter to SolveNonHydro.run_corrector_step
  • (fix) mean_cell_area added to CellParams and removed the calculation from SolveNonHydro.

Cleanups:

  • Cleanup test_solve_nonhydro.py: remove random numbers that were passed instead of the proper vct_a
  • calculation of the Smagorinsky factors is extracted to common and used in diffusion and dycore from there.
  • removed some unsed function in dycore/state_utils/utils.py

halungge added 23 commits October 26, 2023 14:48
@halungge
fix backend imports from gt4py
f8a8011
@halungge
remove unused random values fac, z, a_vec in tests
b4632b6
@halungge
add divdampfac_o2 to argument list
177f213
@halungge
pass entire CellParams to SolveNonHydroInit
7026c10
@halungge
remove non constants params from NhConstants, extract constructor fun…
e503d4b 
…ctions in test
@halungge
remove bdy_divdamp from corrector
1182640
@halungge
add test for enhanced div damping
b10d5f2
@halungge
use mean_cell_area from data
92c6d04 
move divdamp_fac_o2 to corrector
@halungge
fix input values in test_divdamp_factor
e0cd7d6
@halungge
merge main
01d934d
@halungge
merge main
6a0c663
@halungge
fix bounds in predictor tests
f79ab8d
@halungge
add unit tests for divdamp calculation in dycore
f690d36
@halungge
add unit tests for divdamp calculation in dycore
67bc818
@halungge
cleanup logging
e819fdb
@halungge
fuse divdamp calculation
12f3018
@halungge
move enh_smag_fac calculation to common
b9a40e1
@halungge
update serialbox data with mean_cell_area and div_damp_fac_o2
88f6b26
@halungge
use sys.float_info.epsilon for dbl_eps
c152b29
@halungge
fix scal_divdamp in serialbox_utils.py
22fabca
@halungge
Revert "use sys.float_info.epsilon for dbl_eps"
04b89b5 
This reverts commit c152b29.
@halungge
merge main
64a2b6e
@halungge
fix diffusion_wrapper.py: mean cell area
7ce558e
@halungge halungge marked this pull request as ready for review November 15, 2023 09:14
@halungge
Copy link
Contributor Author

cscs-ci run

1 similar comment
@halungge
Copy link
Contributor Author

cscs-ci run

@halungge
Copy link
Contributor Author

launch jenkins spack

@halungge
Copy link
Contributor Author

cscs-ci run

@halungge
Copy link
Contributor Author

cscs-ci run

@halungge
Copy link
Contributor Author

launch jenkins spack

@abishekg7
Copy link
Contributor

launch jenkins spack

@halungge halungge requested a review from abishekg7 November 22, 2023 10:30
abishekg7
abishekg7 approved these changes Nov 22, 2023
View reviewed changes
Copy link
Contributor

@abishekg7 abishekg7 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! It's possible that some assertions fail with dallclose, but we may just need to adjust tolerances

@halungge
Copy link
Contributor Author

cscs-ci run

@halungge
Copy link
Contributor Author

launch jenkins spack

@github-actions GitHub Actions
Copy link

Mandatory Tests

Please make sure you run these tests via comment before you merge!

  • cscs-ci run
  • launch jenkins spack

Optional Tests

In case your change might affect downstream icon-exclaim, please consider running

  • launch jenkins icon

For more detailed information please look at CI in the EXCLAIM universe.

@halungge
Copy link
Contributor Author

cscs-ci run

@halungge
Copy link
Contributor Author

launch jenkins spack

@halungge halungge merged commit b1bfb8a into main Nov 28, 2023
2 checks passed
@halungge halungge mentioned this pull request Dec 1, 2023
Merged
@halungge halungge deleted the fix_solve_nonhydro_divdamp_fac_o2 branch January 10, 2024 10:45
OngChia pushed a commit that referenced this pull request Jan 14, 2025
@halungge
Fix solve nonhydro divdamp fac o2 (#313)
38f8046 
(Fix) 
- pass `divdamp_fac_o2` as argument to `SolveNonHydro.predictor_step` as it changes dynamically in the timeloop
-  use `mean_cell_area` from `CellParams`
- cleanup tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abishekg7 abishekg7 abishekg7 approved these changes

@OngChia OngChia OngChia approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@halungge @abishekg7 @OngChia