V2 configuration intends to use clubb_ipdf_call_placement = 2. But this option would cause restart run to be NBFB.
The issue can be reproduced with the test ERS_Ln5.ne4pg2_ne4pg2.FC5AV1C-L.compy_pgi.eam-thetahy_sl_pg2,, using the branch for #4033 (branch wlin/atm/v2nml_usecase2defaults), or current master with clubb_ipdf_call_placement = 2 set in user_nl_eam.
When the value is '1' (default for current master) or '3', the test would pass.
The NBFB behavior has been produced on compy (pgi) and chrysalis ( intel)
I see the same issue on Chrysalis with a single process job (1x1 PE layout):
-compset FC5AV1C-L -res ne4_ne4
ERS_Ln3 also fails, so we don't need to run 5 time steps (as in ERS_Ln5) to reproduce the failure.
Verified this restart NBFB exist before the merging of #3814 that introduced nml variable clubb_ipdf_call_placement. This nml variable is used to reset the value for a public variable ipdf_call_placement.
Using the commit 32058b8 which was the master before PR #3814, with hardwired ipdf_call_placement = ipdf_post_advance_fields (=2). the NBFB restart behavior was reproduced.
When ipdf_call_placement=2, 0s values could have been used for pdf_params and pdf_params_zm at the 1st step of a restart run. just like at the 1st step of a startup run, passed into advance_clubb_core from clubb_intr. After the first step, the values computed in advance_clubb_core would be passed back to clubb_intr to be used for the next time step. When ipdf_call_placement=2, the two data structures would be used in advance_clubb_core before they are computed. Note that l_use_ice_latent=.false. and l_call_pdf_closure_twice=true if to review the codes.
The two variables get chunk values from public module variables pdf_params_chnk and pdf_params_zm_chnk. They are always initialized to 0s at the beginning of a run. Don't see they are saved to pbuf, and the restart file does not contain related variables. This may confirm indeed 0 values are used at the 1st step of restart, rather than the computed values from previous step.
The two data structure contains a long list of variables, it may not be practical to save them all to restart file. A more feasible fix may be to also call pdf_closure_driver at first_step_restart at the same location as with ipdf_call_placement=1 (pre). Just once.
Note: This is just based on my reading of the codes. Haven't been tested and I could be wrong. Welcome more of us to look into the codes.
Did a test by also calling pdf_closure_driver when is_first_restart_step, still NBFB. It is possible the input clubb state variables to the subroutine are not the same as when calling later in the routine during previous step. Not sure though.
Did a test by also calling pdf_closure_driver when is_first_restart_step, still NBFB. It is possible the input clubb state variables to the subroutine are not the same as when calling later in the routine during previous step. Not sure though.
Hi @wlin7 , My feeling is that whenever pdf_closure_driver is called, the clubb variables are updated. Also, clubb is running for several sub-time steps within each model time step. Should we involve clubb developers in this hunting game at this point? Thanks
Message from Brian Griffin
When the PDF is in the "post" position (ipdf_call_placement = 2), you will need to save the "thvp" family of variables from timestep-to-timestep (so, that's wpthvp, wp2thvp, rtpthvp, and thlpthvp). There are also some additional variables that need to be saved, depending on model flag settings.
Most directly relevant is the l_call_pdf_closure_twice flag. This is automatically turned on when l_vert_avg_closure is turned on. When this flag is enabled, the following variables from type pdf_params_zm_chnk would need to be written to the restart file: %w_1, %w_2, %varnce_w_1, %varnce_w_2, and %mixt_frac.
Additionally, in the scenario that l_avg_Lscale and l_Lscale_plume_centered are both turned on, the following variables from type pdf_params_chnk would need to be written to the restart file: %rt_1, %rt_2, %varnce_rt_1, %varnce_rt_2, %thl_1, %thl_2, %varnce_thl_1, and %varnce_thl_2.
Should we involve clubb developers in this hunting game at this point?
Yes, @xuezhengllnl , as you can see above, Brian and Vince are fully aware of the situation and helping with this issue.
Possible path forward: (WIP)
As Brian outlined, 17 3D fields need to be carried over via restart file to get a proper BFB restart check point. Four of them already in restart file (wpthvp, wp2thvp, rtpthvp, and thlpthvp), automatically handled by pbuf after registering with pbuf2d.
That leaves 13 other fields, for members in the structure variable pdf_params_chnk and pdf_params_zm_chnk; and that is 9 out of 47x2 variables in the two data structure, thus manageable by registering them with pbuf, then restart file as needed.
In clubb_tend_cam, we need 13 pointers for these member fields t, like other fields from pbuf to be associated with pbuf chunk by chunk. This should happen every step. When is_first_restart_step, these 13 fields must be associated with (or assigned to) the corresponding members in the data structure pdf_params and pdf_params_zm. The return values after calling advance_clubb_core should also be duplicated to the 13 variables, in addition to them being associated with pdf_params_chnk and pdf_params_zm_chnk column by column for each chunk.
@worleyph, can you please advise how to properly manage the chunks? No chunk info is involved when calling pbuf_get_field, while pdf_params_chnk and pdf_params_zm_chnk gather data for all the chunks. How to match the data at chunk level? [Correction: pbuf seen by clubb_tend_cam is already at chunk level, so I think if to send pbuf data to variable like pdf_params, just use lchnk carried by the state variable. It is already used in that format in the subroutine]
[updated: 13 instead of 9 variables if both pdf_params and pdf_params_zm are invoved. @bmg929 determined only 5 needed for current E3SM configuration - see below]
Adding the 9 3D fields above on physics grid is expected to increasse eam.r file size by 3%.
Message from Brian Griffin
Additionally, in the scenario that l_avg_Lscale and l_Lscale_plume_centered are both turned on, the following variables from type pdf_params_chnk would need to be written to the restart file: %rt_1, %rt_2, %varnce_rt_1, %varnce_rt_2, %thl_1, %thl_2, %varnce_thl_1, and %varnce_thl_2.
I don't believe that E3SM uses this configuration, does it? If it's not something that E3SM would ever turn on, perhaps these variables could be omitted if it would simplify the situation.
Then, the only additional variables that you would need to save are the 5 that are associated with the pdf_params_zm_chnk variable.
@wlin7 , I have never looked carefully into the I/O logic dealing with "chunked" data structure. I think that the variables are registered as chunked or not, and then everything is taken care of internally? Will need to know the chunk id though, but based on your correction, this information is available?
Yes, Pat. I think we have the needed chunk information.
Then, the only additional variables that you would need to save are the 5 that are associated with the
pdf_params_zm_chnkvariable.
Oh, thanks @bmg929. That is better. I was looking at the inner block, conditioned by l_use_ice_latent, which currently set to false, that else block invokes pdf_params.
Oh, thanks @bmg929. That is better. I was looking at the inner block, conditioned by l_use_ice_latent, which currently set to false, that else block invokes pdf_params.
Since pdf_params_zm_chnk is a module variable, the information should be saved from timestep-to-timestep. The only concern is writing it to a restart file. Perhaps 5 new variables could be added to the pbuf (e.g. clubb_pdf_zm_w_1, clubb_pdf_zm_w_2, clubb_pdf_zm_varnce_w_1, clubb_pdf_zm_varnce_w_2, and clubb_pdf_zm_mixt_frac, or whatever you think an appropriate name would be), and then they could be set equal to the appropriate value found in pdf_params_zm_chnk sometime after the call to advance_clubb_core.
e.g.
clubb_pdf_zm_w_1 = pdf_params_zm_chnk%w_1
Then, if it's a restart, you could have a statement that runs the opposite way sometime before the call to advance_clubb_core.
Yes, @bmg929 , we will only add the 5 variables to pbuf. The returned values for these 5 variables from advance_clubb_core will be passed to the 5 pbuf variable every step. pbuf to pdf_params_zm_chnk will only be done on first_restart_step.
The naming for the new pbuf variables very informative. I will use them -- but I will check if there is a length limit.
It has already been diagnosed by @wlin7 and @bmg929 but the perturbation growth test method also pointed to the same variables:
w_1_zm
w_2_zm
varnce_w_1_zm
varnce_w_2_zm
mixt_frac_zm
They all are derived from pdf_params. The first difference shows up when we compute low_lev_effect and high_lev_effect using subroutine calc_turb_adv_range as all the above variables are zero in the restart run.
Hi @singhbalwinder , this perturbation growth test method is very cool. When I did the first analysis of the codes, I saw pdf_params_chnk and pdf_params_zm_chnk appeared to be always initialized to 0s in clubb_ini_cam, restart or startup run. I could only suspect 0 values have been used because I only trace a few module files and did't check them in runtime.
BTW, just to add a note that since our current config has l_call_pdf_closure_twice = true, the five variables above use the values from pdf_params_zm. When l_call_pdf_close_twice=false, indeed they will use interpolated values from pdf_params.
Thanks to @bmg929 and @vlarson as well as @singhbalwinder for providing/confirming a precise list of variables needed to be in the restart file. We now have a fix (PR #4071) for this issue.
Sorry to report we are not out of the woods yet. While ERS tests now pass, ERP tests (performing restart, and also changing pe-layout) continue to fail. The test I did is ERP_Ln9.ne4_ne4.FC5AV1C-L.compy_pgi. No clue yet. Pinging @worleyph , @singhbalwinder , @ambrad for help. Please use the branch for #4071 (wlin/atm/fix_restnbfb_clubbpdfcallpl2), and add clubb_ipdf_call_placement=2 in user_nl_eam. Thanks.
Please wait. It is likely a false alarm. I was not fully aware of the ERP test structure, causing the settings for clubb_ipdf_call_placement not done properly for the two runs in the test. To update soon.
Alarm lifted. The ERP test passed when clubb_ipdf_call_placement=2 was set in both case and case2 directories. I missed the case2 previously when I didn't know how the ERP test is managed. Note that need to manually reset this parameter for the test because #4033 is not in yet.
A side note: I was not able to use the intended field names like clubb_pdf_zm_varnce_w_1, clubb_pdf_zm_varnce_w_2 in the restart file as suggested by @bmg929 . cam_pio_def_var would resolve them to be an identical truncated substring and fail. A length limit for the field name must be in play. Shorter names like pdf_zm_var_w_1 and pdf_zm_var_w_2 are used.
Most helpful comment
It has already been diagnosed by @wlin7 and @bmg929 but the perturbation growth test method also pointed to the same variables:
They all are derived from
pdf_params. The first difference shows up when we computelow_lev_effectandhigh_lev_effectusing subroutinecalc_turb_adv_rangeas all the above variables are zero in the restart run.