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

Remove lhdiff_rcf from solve_nonhydro config #431

Merged
merged 3 commits into from
Apr 5, 2024
Merged

Remove lhdiff_rcf from solve_nonhydro config #431

merged 3 commits into from
Apr 5, 2024

Conversation

OngChia
Copy link
Contributor

@OngChia OngChia commented Apr 4, 2024

I remove the option lhdiff_rcf from solve_nonhydro and diffusion config because diffusion should only be applied at the full time step, and thus it is redundant after discussion with Anurag.

… is by default True and redundant after discussion with Anurag
@OngChia OngChia changed the title Remove lhdiff_rcf from diffusion and solve_nonhydro config Remove lhdiff_rcf from solve_nonhydro config Apr 4, 2024
@OngChia
Copy link
Contributor Author

OngChia commented Apr 4, 2024

cscs-ci run default

@OngChia
Copy link
Contributor Author

OngChia commented Apr 4, 2024

launch jenkins spack

@halungge halungge self-requested a review April 4, 2024 15:04
@halungge halungge self-requested a review April 4, 2024 15:12
Copy link
Contributor

@halungge halungge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is actually also still in diffusion.py, where it is also named lhdiff_rcf I searched for apply_horizontal_diffusion_at_large_dt before. Can you remove it there as well?

Copy link

github-actions bot commented Apr 5, 2024

Mandatory Tests

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

  • cscs-ci run default
  • launch jenkins spack

Optional Tests

To run benchmarks you can use:

  • cscs-ci run benchmark

To run tests and benchmarks with the DaCe backend you can use:

  • cscs-ci run dace

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.

@OngChia
Copy link
Contributor Author

OngChia commented Apr 5, 2024

Yes. I have removed lhdiff_rcf in diffusion. I also removed it in wrapper module for diffusion granule tools icon4pytools/py2fgen/wrappers/diffusion.py. I think it should be okay for blue line.

@OngChia
Copy link
Contributor Author

OngChia commented Apr 5, 2024

launch jenkins spack

@OngChia
Copy link
Contributor Author

OngChia commented Apr 5, 2024

cscs-ci run default

@OngChia OngChia requested a review from halungge April 5, 2024 07:38
@OngChia OngChia merged commit 96755a5 into main Apr 5, 2024
5 checks passed
iomaganaris referenced this pull request in iomaganaris/icon4py Jun 18, 2024
* Remove lhdiff_rcf from diffusion and solve_nonhydro config because it is by default True and redundant after discussion with Anurag

* Remove lhdiff_rcf in diffusion. lhdiff_rcf in py2fgen has to be removed too.
@samkellerhals samkellerhals deleted the lhdiff_rcf branch December 20, 2024 08:36
OngChia added a commit that referenced this pull request Jan 14, 2025
* Remove lhdiff_rcf from diffusion and solve_nonhydro config because it is by default True and redundant after discussion with Anurag

* Remove lhdiff_rcf in diffusion. lhdiff_rcf in py2fgen has to be removed too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants