NOAA-GFDL / GFDL_atmos_cubed_sphere

The GFDL atmos_cubed_sphere dynamical core code
Other
56 stars 118 forks source link

Redesign saturation table initialization #303

Closed laurenchilutti closed 10 months ago

laurenchilutti commented 10 months ago

Description

Bringing updates made by Linjiong Zhou to the main branch. Will merge to the dev/gfdl_am5 branch.

Call qs_init in the AM model when the GFDL MP isn't used. Remove the redundant qs_init in the GFDL MP.

Fixes # (issue)

How Has This Been Tested?

Tested by FV3 Team developers.

Checklist:

Please check all whether they apply or not

bensonr commented 10 months ago

@laurenchilutti - has this been previously fixed in the SHiELD driver?

linjiongzhou commented 10 months ago

@bensonr, this fix is designed for the AM model. We have gfdl_mp_init in the SHiELD driver, so we don't need qs_init.

laurenchilutti commented 10 months ago

@bensonr To summarize the detail Linjiong gave me:

In AM5, when the relative humidity diagnostic was turned on, the model blew out with a segmentation fault error. This error pointed to the GFDL MP saturation water vapor table.

It turns out that the saturation water vapor table wasn't initialized in AM5, and our original code to fix this with "if (.not. tables_are_initialized) call qs_init" was useless within AM5.

Therefore, Linjiong proposed calling qs_init in AM5 during the initialization, similar to the procedure for calling gfdl_mp_init during initialization in SHiELD. Baoqiang completed testing on this approach and confirmed that it resolved the issue.

bensonr commented 10 months ago

@laurenchilutti @linjiongzhou - wouldn't it have been easier to make sure it was initialized from within fv_diagnostics? qs_init is already included via module use within fv_diagnostics and it's a matter of expanding what diag ids require it. I'm also assuming the tables are initialized once and have protections about them. It's possible we could remove the #if/#endif pair and always make sure the tables are initialized.

linjiongzhou commented 10 months ago

@bensonr, the saturation table is not only used in the diagnostic but also in other places that involve saturation calculation. For example, the 2dx filter and negative tracer removal use the saturation table. That is why I intended to put it in the initialization process.

bensonr commented 10 months ago

@linjiongzhou - _fv_diaginit comes 12 lines after the existing qs_init in _atmosphereinit. If the qs_init is not needed by the fv_restart process, then it'll be initialized by the diagnostics. If you want to always initialize _qsinit in _atmosphereinit, then let's consider removing the one fv_diagnostics_init.

laurenchilutti commented 10 months ago

@bensonr Let me know if this is ready to merge, or if you would like @linjiongzhou to make changes

linjiongzhou commented 10 months ago

@bensonr, thanks for your suggestion. I can take the "call qs_init" off the fv_diag_init. It is safer to put it in the atmosphere_init. Thanks!

bensonr commented 10 months ago

@linjiongzhou - wfm. I'll approve this one now and we'll fix the diagnostics in another PR