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

updates needed for verbose diagnostic output #534

Closed
10 of 11 tasks
eclare108213 opened this issue Nov 26, 2020 · 3 comments
Closed
10 of 11 tasks

updates needed for verbose diagnostic output #534

eclare108213 opened this issue Nov 26, 2020 · 3 comments
Assignees
Labels

Comments

@eclare108213
Copy link
Contributor

eclare108213 commented Nov 26, 2020

I have scanned the ‘verbose’ output in the log files for base_suite. Here are some suggested updates:

  • move bathymetry_format one space to the left
  • kevp_kernel=0 should be labeled “original EVP solver”, correct? And the other value (in alt01) is “vectorized EVP solver”?
  • do we need both “advection” and “ktransport” once upwind is deprecated?
  • atmiter_conv appears to be 0 all the time. Maybe the formatting should be the same as is used for dSdT_slow_mode.
  • new namelist variables should be added to the verbose diagnostic prints, e.g. floeshape, floediam, hfrazilmin?
  • I don’t think close_boundaries should be printed, since there’s not really an option there. (?)
  • sw_redist, sw_frac, sw_dtemp should be listed under Radiation instead of Thermodynamics, and sw_frac and sw_dtemp should only be listed when sw_redist=T.
  • Many of our tests aren’t highlighted in the verbose diagnostics (i.e., the verbose diagnostics are incomplete)
  • The VP namelist parameters should be added to the verbose diagnostics when kdyn=3 (and printed depending on the configuration e.g. fgmres, pgmres etc)
  • Nothing is printed when thermo is turned off. When ktherm=-1, print “Thermodynamics off”, and don’t print the Radiation block since it's irrelevant. (e.g. gbox80_1x1_box2001)
  • ns_boundary_type formatting is bad for tx1_40x4_dsectrobin_medium - wasn’t this fixed already?
@apcraig apcraig added the IO label Dec 3, 2020
@apcraig apcraig self-assigned this Dec 7, 2020
@apcraig
Copy link
Contributor

apcraig commented Dec 7, 2020

Working on a branch called swnml to fix these issue. Many are straight-forward. I have a sandbox and will create a PR soon to support further discussion. A few questions,

  • "verbose diagnostics are incomplete". Are there some obvious things we know we need to add? The alternative is to go thru all the namelist and figure out which we're not writing out. Suggestions?

  • The vp namelist were printed but in the "other namelist" section. I have moved them into the dynamics. It could be that more "if" checks need to be added for printing. If we want to do that, some guidance would be helpful about how to organize the vp namelist into appropriate groups.

  • The ns_boundary_type is not yet defined in ice_init. It is read in domain_nml which happens later. I have removed the ns_boundary_type output. It is written out to the log file when it's read in later. If we need to move ns_boundary_type from domain_nml to grid_nml, let me know. But I recommend against that (for backwards compatibility) at this point. For now, I'll leave it in domain_nml.

@eclare108213
Copy link
Contributor Author

Cool, thanks.

  • "verbose diagnostics are incomplete". Are there some obvious things we know we need to add? The alternative is to go thru all the namelist and figure out which we're not writing out. Suggestions?

This is not necessarily something we need to fix right away, but if it's easy then we could make some headway. I noticed it because I was only reading through the verbose diagnostics section of the log files when checking whether the various base_suite settings made sense with one another, and in some cases, there was no indication in that section of what the test was doing - it only listed the default settings, so I had to diff the log file with one from a 'real' default run (or else look at Tony's spreadsheet). This is not surprising, since I prioritized what went into the verbose section based on what I thought a user writing a general-purpose description of the model would want to include in a journal article. This is low priority IMHO, although it means I might have missed some conflicting physics in my review.

  • The vp namelist were printed but in the "other namelist" section. I have moved them into the dynamics. It could be that more "if" checks need to be added for printing. If we want to do that, some guidance would be helpful about how to organize the vp namelist into appropriate groups.

Maybe @JFLemieux73 and @phil-blain can help with this. There are some parameters that are only used for VP, some only for EVP or EAP, some maybe for both...

  • The ns_boundary_type is not yet defined in ice_init. It is read in domain_nml which happens later. I have removed the ns_boundary_type output. It is written out to the log file when it's read in later. If we need to move ns_boundary_type from domain_nml to grid_nml, let me know. But I recommend against that (for backwards compatibility) at this point. For now, I'll leave it in domain_nml.

Agreed.

@apcraig
Copy link
Contributor

apcraig commented Dec 7, 2020

There is a draft PR, #540 with these changes. I am testing now. But I am also happy to make additional changes.

With regard to the "verbose diagnostics incomplete". It sounds like it might be helpful to put a * (or something) next to namelist that are not set at default values. That would take quite a bit of work and I think should be a standalone effort. Or am I missing the point?

The vp outputs are pretty well highlighted in the diff. As I said, I moved them from "other" to dynamics. I think they are only written if kdyn=3. Again, happy to introduce additional refinements if it's helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants