E3SM-Project / E3SM

Energy Exascale Earth System Model source code. NOTE: use "maint" branches for your work. Head of master is not validated.
https://docs.e3sm.org/E3SM
Other
332 stars 334 forks source link

reduce the warning printing from the atm chemistry implicit solver #6380

Open keziming opened 2 weeks ago

keziming commented 2 weeks ago

This PR is to reduce unnecessary warning messages from the atm chemistry implicit solver. Background: The implicit solver for atmospheric chemistry prints out warning messages every time at each location when the solver can't converge at the current delta-t. For most times, the convergence issue could be resolved by reducing the chemistry solver delta-t. Therefore, these messages can be overwhelming and bury other important messages.

Modifications: For production runs, we don't need these messages if the implicit solver can converge by reducing the delta-t two times (halving delta-t each time). We removed the message at line 530, which is responsible for the excessive warnings and duplicated the information especially for the production runs. Additionally, we lift the threshold to report the convergence issue from 0 to 2.

Test results: The test and control runs are BFB according to the 10-day simulation. There is no clear throughput change between the two simulations.

Control run /lcrc/group/e3sm/ac.zke/E3SMv3_dev/20240502.v3.control.F2010-TMSOROC002-Z025-DUST113.ne120pg2_r05_icos30.chrysalis/tests/custom-22_1x10_ndays/run

Test run /lcrc/group/e3sm/ac.zke/E3SMv3_dev/20240501.v3.test.F2010-TMSOROC002-Z025-DUST113.ne120pg2_r05_icos30.chrysalis/tests/custom-22_1x10_ndays/run

github-actions[bot] commented 2 weeks ago

PR Preview Action v1.4.7 :---: :rocket: Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-6380/ on branch gh-pages at 2024-05-03 04:01 UTC

rljacob commented 2 weeks ago

Which implicit solver? There is more then one in E3SM.

keziming commented 2 weeks ago

Which implicit solver? There is more then one in E3SM.

@rljacob the implicit solver for solving chemistry reactions.

mahf708 commented 2 weeks ago

I am not sure if it is good practice to be changing auto-generated code (this code is autogenerated by the chem pp).

For additional context, something along these lines came up at least twice before. Here's the most recent one: https://github.com/E3SM-Project/E3SM/pull/5890

tangq commented 2 weeks ago

I don't like directly changing the auto-generated code either as I indicated at multiple occasions. This PR is a quick fix after discussing with the coupled group leader to reduce unnecessary prints (and potential slowdown) in the ongoing v3.HR and v3.NARRM developments, considering E3SM now has the final v3.LR and is transitioning from fortran to c++ code base.

For a long-time fix, changes should be done in the pre-proccesor (PP) at components/eam/chem_proc/procfiles/cam/mo_imp_sol_cache.F90. Note that PP is used to generate codes for all chemistry mechanisms including testing new ones, therefore, the broader impacts need to be assessed when modifying PP. I am not sure the coupled group wants to pursue this route now.

keziming commented 2 weeks ago

@wlin7 @crterai @rljacob @tangq @xuezhengllnl After test, this PR is BFB and effectively reduced imp_sol warning prints