Correct comments describing generic_tracer args#44
Merged
marshallward merged 2 commits intoDec 24, 2021
Conversation
Corrected uninformative comments describing the some of the arguments to the stub routines in config_src/external/GFDL_ocean_BGC/generic_tracer.F90. The updated comments are consistent with how they are used in calls to these routines and with the underlying actual generic_tracer code if they are actually documented there. The previous comments had been added to existing undocumented code to satisfy the MOM6 requirement that there be a doxygen comment describing every argument to every routine, in the hopes that someone with familiarity with the generic tracer could work amend them to something more appropriate. However, "Unknown" is neither an accurate nor an informative description, and current MOM6 standards would demand that we reject any new code contributions with such poor interface documentation. All answers are bitwise identical, and only comments have changed.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #44 +/- ##
=========================================
Coverage 28.96% 28.96%
=========================================
Files 242 242
Lines 71324 71324
=========================================
Hits 20660 20660
Misses 50664 50664
Continue to review full report at Codecov.
|
Member
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/14421 ✔️ |
marshallward
approved these changes
Dec 24, 2021
This was referenced Jan 29, 2022
uwagura
pushed a commit
to uwagura/MOM6
that referenced
this pull request
Nov 4, 2025
* Vertvisc: First part of loop Moving gradually * add nvtx ranges and module warpper * port loop to openmp for now -> will move to do concurrent. Focused on porting to GPU now * more gpu * port the vertical velocity to GPUs * port the following vertical velocity loops to GPUs * more loops using omp target teams, validated * fix compute sanitizer death * meridional velcoity component ported * finish porting vertvisc * offload another loop * revert the last thing * add collapse(2) to the expensive loops * fix * fix validation * updates to porting: find coupling coef * almost done, just missingt he killer loop * lable the block of death * duct taped version of all funcs ported to GPUs * back to format and delete block constructs * remove the maps ins remnant yay! * remove need for local vertvisc_u/v variables * death loop of death has been ported * vertvisc coef does not have explicit mappings anymore * the maps are gone! * limit vel using single data transfer * do concurrent * some memory cleanup and code cleanup - profiling time * update and lots of nvtx markers for opt * limit vel betterments and notes for optimization * make limit vel good * h_ml transfer * Vertfrict: Move CS allocations to init Allocations of the vert frictions and visc control structures were moved from the dycore subroutine loops to the model initialization. Redundant grid (G) transfers were removed (although they were not triggering any transfers). Most (but not all) visc arrays have also been moved into the initialization. Kv_shear and Ray_[uv] need to be added. Trailing whitespace also (inadvertently) got removed, hopefully not a distraction. * coupling coeff fixes This appears to improve the CPU/GPU repro of the latest merge of dev/gpu into dev_gpu_vertvisc. * Resolves some of the u/v/h/dz field states between the barotropic and vertvisc functions. (Development of each assumed that the other was not complete). * This seems to fix some issues with a_[uv], Kv_tot, and dz inside of find_coupling_coef and vertvisc coef * chksum transfers were added to ensure consistent * The diff has moved to frhat[uv] in btcalc. The fields are all zero on GPU, so they are probably not transferred. Work in progress... * Remove redundant BT_cont transfer The BT_cont fields are now on GPU, so they don't need to be transferred before the btcalc call. Checksum transfers for h_ml were also added. Still assuming that it's computed on the CPU. * delete nvtcx * test to see if guarding and reintroducing the check helps * an ugly fix for a simple problem: just want to be sure before I dive * spaces * Vertvisc: formatting/style cleanup This patch reduces the formatting changes to this branch and brings it closer to both dev/gfdl and the MOM6 style guide. One minor non-cosmetic change is the reversion of ADp%dv_dt_str(i,J,1) calculation into one of the main Thomas loop. This returns it to a separate loop. * Vert friction: Explicit CS alloc in dycore init This patch removes the NVIDIA compiler preprocessing and explicitly compiles the `CS` in the dynamic core initilization. `vertvisc_init` is no longer responsible for allocation. The allocation test has also been removed. We just have to trust each other now (or segfault when it refs an unallocated field). The previous error in the CI was the absense of `CS` allocation in the other timestep methods (unsplit, unsplit RK, split RK2b). * Vert friction: Data transfer reduction Several modifications to the data transfer statements * Some redundant transfers were removed * Input/output transfers were moved up to the dycore loop * Vert friction: Cosmetic cleanup Remove some redundant comments, and an unused array loop. * Vert friction: nk_in_ml bugfix --------- Co-authored-by: Marshall Ward <marshall.ward@noaa.gov>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Corrected uninformative comments describing the some of the arguments to the
stub routines in config_src/external/GFDL_ocean_BGC/generic_tracer.F90. The
updated comments are consistent with how they are used in calls to these
routines and with the underlying actual generic_tracer code if they are actually
documented there. The previous comments had been added to existing undocumented
code to satisfy the MOM6 requirement that there be a doxygen comment describing
every argument to every routine, in the hopes that someone with familiarity with
the generic tracer could work amend them to something more appropriate.
However, "Unknown" is neither an accurate nor an informative description, and
current MOM6 standards would demand that we reject any new code contributions
with such poor interface documentation. All answers are bitwise identical, and
only comments have changed.