MOM6: +(*)Rescale pressures#1089
Merged
marshallward merged 62 commits intoApr 21, 2020
Merged
Conversation
Added optional pres_scale arguments to various MOM_EOS.F90 routines to allow pressures to be passed with in various units for flexibility and streamlined dimensional consistency testing. Also added optional rho_scale arguments to the various int_density_dz subroutines. All answers are bitwise identical, but there are new optional arguments to public interfaces.
Rescaled the pressures in Boussinesq pressure force calculations, including changing the units of the densities passed to set_pbce_Bouss and using the new rho_scale and pres_scale arguments to the equation of state routines. All answers are bitwise identical.
Optionally rescale the units of the specific volume integrals. Added new optional SV_scale and pres_scale arguments to int_specific_vol_dp, int_spec_vol_dp_generic, and int_spec_vol_dp_Wright. All answers are bitwise identical, but there are new optional arguments to public interfaces.
Rescaled the pressures and specific volumes in the non-Boussinesq pressure force calculations, including changing the units of the pressures passed to set_pbce_nonBouss and using the new SV_scale and pres_scale arguments to the equation of state routines. All answers are bitwise identical.
Rescaled the pressure used to calculate density integrals in find_eta_3d and find_eta_2d to [R L2 T-2] and used the new pres_scale and SV_scale arguments to int_specific_vol_dp. All answers are bitwise identical.
Rescaled the pressure used in calls to calculate_density_derivs to [R L2 T-2] in calculate_density_derivs. All answers are bitwise identical.
Rescaled pressures to [R L2 T-2] in calculate_vertical_integrals for improved dimensional consistency testing. All answers are bitwise identical.
Added new RL2_T2_to_Pa and W_m2_to_RZ3_T3 elements to the unit_scale_type for code simplification and clarity. Also corrected spelling errors in the get_param descriptions of 5 scaling factors, which will change comments in the MOM_parameter_doc.debugging files. All answers are bitwise identical.
Replaced products of scaling factors (like US%R_to_kg_m3*US%L_T_to_m_s**2) with combined scaling factors (like US%RL2_T2_to_Pa) to simplfy and clarify the code. All answers are bitwise idenical.
Rescaled the units of forces%p_surf, fluxes%p_surf, forces%p_surf_full and fluxes%p_surf_full and related surface pressure variables to [R L2 T-2 ~> Pa] for expanded dimensional consistency testing. All answers are bitwise identical, although there are changes to the rescaled units of elements to two transparent data types.
Revised the dimensional rescaling of ice_shelf_CS%g_Earth to match GV%g_Earth and ice_shelf_dyn_CS%g_Earth to minimize confusion when examining different parts of the code. Also cancelled out pairs of unit conversion factors when setting the ice shelf contributions to fluxes%p_surf and forces%p_surf. All answers are bitwise identical.
Added a new optional pres_scale argument to the calculate_TFreeze interfaces to rescale pressures for dimensional consistency testing. All answers are bitwise identical.
Use the new pres_scale argument to TFreeze and pass rescaled pressures to calculate_TFreeze and several instances of calculate_density. All answers are bitwise identical.
Added variants of calculate_density routines that use a hor_index_type to specify array extents and unit_scale_types for dimensional consistency testing, further overloading existing interfaces. Also replaced the recently added rho_scale and pres_scale arguments to int_density_dz with an optional unit_scale_type argument, and modified calls to use this new argument. All answers are bitwise identical, but there are changes to external interfaces.
Revised numerous calls to calculate_density and calculate_density_derivs to use the new form with domain extents indicated by a hor_index_type argument. Internal density variables were also rescaled in a few cases. All answers are bitwise identical.
Rescaled the units of tv%P_Ref to [R L2 T-2] for expanded dimensional consistency testing. In some cases, other pressure variables were also rescaled and calls to calculate_density are recast into the simpler G%HI forms. All answers are bitwise identical, but the scaled units of an element of a transparent type were rescaled.
Rescaled the reference pressure arguments to 3 set_coord routines, 5 initialization routines, and kappa_shear_column. Also removed the unused pres argument to convert_temp_salt_for_TEOS10 and replaced its ocean_grid_type argument with a hor_index_type. All answers are bitwise identical.
Replaced the remaining pres_scale arguments the various calculate_density and calc_spec_vol routines in MOM_EOS.F90 with new optional unit_scale_type arguments. When the scale and US arguments are present, density is scaled by the product of the indicated scaling factors. Calls to these routines in 11 files were modified accordingly. All answers are bitwise identical.
Corrected pressure unit documentation in comments in 5 files. Also fixed punctuation in comments in MOM_EOS.F90. All answers are bitwise identical.
Rescaled internal pressure, specific volume and energy variables in MOM_diapyc_energy_req.F90. This file is mostly used for testing, and all answers are bitwise identical.
Rescaled pressures in 17 calculate_density or related calls and internal pressure variables in MOM_diabatic_aux. All answers are bitwise identical.
Corrected a rescaling bug in calculate_spec_vol_derivs_H1_1d, but as this code was not yet being exercised, there are no answer changes. Also modified applyBoundaryFluxesInOut to use this fixed routine. All answers are bitwise identical.
Redirected complicated equation of state routines to all work via the same 1-d array versions of the code. This shortens the MOM_EOS.F90 code even as new routines are added under the same overloaded interfaces. All answers are bitwise identical.
Rescaled the units of the pressure, density, and gravitational acceleration arguments to find_depth_of_pressure_in_cell, frac_dp_at_pos, trim_for_ice and cut_off_column_top for dimensional consistency testing and code simplification. One change corrected an omitted scaling factor when the optional z_tol argument is omitted, but this does not impact any solutions as this argument is present in all active cases. Also cleaned up some bizarre error messages starting with 'Blurgh!', which is apparently an artificial expletive invented to circumvent censors, but has now been censored and replaced with a more informative message. All answers are bitwise identical, but the units of some arguments have changed and there are new unit_scale_type arguments to some routines.
Made both arguments to unit_scaling_init optional to enable the use of this routine to initialize unscaled unit_scale_types for certain types of unit testing. All answers are bitwise identical.
Rescaled pressure and density variables in MOM_neutral_diffusion and added numerous comments describing internal variables and their units. Some unused variables were deleted, including unused pressure arguments to find_neutral_pos_linear. In addition unit_scale_type arguments were added to 8 subroutines, including neutral_diffusion_init and tracer_hor_diff_init. All answers are bitwise identical, but there are changes to public interfaces.
Use the US form of calculate_density calls in ISOMIP_initialization. All answers are bitwise identical.
Nullified a pointer used in neutral diffusion unit tests. Without this correction of a problem introduced two commits ago, these unit tests would sometimes fail, but all solutions are bitwise identical.
Rescaled 3 diagnosed densities and the pressures used to calculate them. All answers are bitwise identical.
- As part of the process of disconnected EOS from the unit_scaling_type this adds the necessary unit conversions to the EOS_type. - Initialization is currently donne by passing US to MOM_init() but ultimately it seems passing p_scaling, etc., to MOM_init() would remove all dependency on US. - No APIs other than EOS_init() have been changed yet.
Use the new variant of calculate_density with the 'dom' argument or no array extent argument in calls in 15 files. All answers are bitwise identical.
- Removes US as an argument wherever it was optional since the unit conversion factors are not stored in the EOS type.
- Resolved conflicts
Applied dimensional rescaling to many of the internal calculations in the 4 MOM_CVMix files, although calls to external CVMix routines still use the original MKS units. These changes include rescaling of the input and output variables associated with the calculate_density routines. All answers are bitwise identical.
Rescaled internal density and pressure variables in coord_adapt, as well as some input parameters. These changes include rescaling of the input and output variables associated with the calculate_density routines. One variable that was being reused with different units has been split into two, and there are new arguments to build_grid_adaptive, build_adapt_column, and init_coord_adapt. All answers in the MOM6-examples test suite are bitwise identical.
Changed to the new interfaces for calculate_density and related calls in 22 places. All answers are bitwise identical.
… Adcroft_rescale_pressure Merge branch 'rescale_pressure' of git://github.com/adcroft/MOM6. This merge compiles, but it fails the dimensional consistency testing and will be fixed shortly.
Corrected problems with dimensional consistency testing in MOM_EOS.F90 that had been introduced with a recent merge. All answers are bitwise identical and are once again passing dimesional consistency testing.
Eliminated the US argument to find_depth_of_pressure_in_cell, which was no longer being used. Also stored EOS_domain values in MOM_state_initialization for reduced overhead. All answers are bitwise identical.
Store the return values from EOS_domain for computational efficiency. Also cleaned up unneeded 'dom=' declarations in compute_density calls. All answers are bitwise identical.
Corrected a diagnostic halo extent in a recently added EOS_domain call and added new variables to two openMP directives. All answers are bitwise identical.
Eliminated the use GV%mks_g_Earth throughout the MOM6 code. This variable is being retained and is still set to avoid breaking any user code that might be using it. All answers are bitwise identical.
Collaborator
Author
|
The 22 recent commits added to this PR address all of the concerns raised in reviews of the earlier version of his PR. The additional commits that were added to this PR to address these issues include:
This revised PR is being tested with https://gitlab.gfdl.noaa.gov/ogrp/MOM6/pipelines/10226. |
Corrected compile time errors, related to an incomplete implementation of the pressure rescaling in mom_surface_forcing_mct.F90. With this fix, the changes relative to dev/gfdl are now similar between mom_surface_forcing_mct.F90 and the equivalent files for the other couplers. All answers in the MOM6-examples test cases are bitwise identical.
adcroft
approved these changes
Apr 20, 2020
Collaborator
adcroft
left a comment
adcroft
left a comment
There was a problem hiding this comment.
This is basically ready to go except @Hallberg-NOAA nad I just discussed a small cleanup in neutral diffusion. Needs to be handed manually due to parameter doc changes.
Removed unused US arguments to routines in MOM_neutral_diffusion.F90. All answers are bitwise identical, but some internal interfaces have one fewer argument, and have been returned to their form in the dev/gfdl version.
adcroft
approved these changes
Apr 20, 2020
Collaborator
adcroft
left a comment
adcroft
left a comment
There was a problem hiding this comment.
@marshallward Can you take this from here? MOM6-examples will have new doc files (.debugging) so needs a manual merge.
Collaborator
|
Updated Gaea regression test: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/pipelines/10234 Passed with no regressions (up to parameter docs), I will merge this now. |
herrwang0
pushed a commit
to herrwang0/MOM6
that referenced
this pull request
Apr 24, 2026
…om-ocean#1089) * Vendor sphinxcontrib-autodoc_doxygen into docs/_ext/ This is piece 1 of the sphinx toolchain upgrade. The docs build previously depended on four forks which haven't been updated since 2020. The chain pinned the entire build to Sphinx 3.2.1, requiring a growing list of transitive version ceilings (jinja2<3.1, sphinxcontrib_*<1.0.x, alabaster<0.7.14, setuptools<82). This commit handles the sphinxcontrib-autodoc_doxygen dependency. The upstream (rmcgibbo) has been dormant since June 2021; the fork was effectively a rewrite (~90% of xmlutils.py changed, both documenters replaced, the whole domain switched from cpp to f). Monkey-patching the upstream package was rejected because there was nothing stable to patch against. Instead the fork's code is vendored in-tree at docs/_ext/autodoc_doxygen/ so it can be debugged, blamed, and edited like any other project source. Changes vs the fork at tag 0.7.13: - Renamed doxynamespace.rst template to doxymodule.rst and dropped the unused C++ doxyclass.rst (DoxygenClassDocumenter was already unregistered in the fork). - Removed ~80 lines of dead code: commented-out pdb.set_trace lines, the unregistered DoxygenClassDocumenter class, the dead visit_ref_angus alternative, _import_by_name_original, and the unused try-import of flint. - Dropped Python 2 compatibility cruft: from __future__ imports, from six import itervalues (replaced with dict.values()). - Ported four Sphinx 3 -> 8 API changes: * DoxygenAutosummary.get_items now passes self.bridge (the DocumenterBridge) to documenter constructors instead of self, so Documenter.__init__ finds directive.genopt. * self.warn / self.directive.warn calls (removed in Sphinx 8) replaced with sphinx.util.logging.getLogger(...).warning. * Wrapped env.doc2path in os.fspath to silence the RemovedInSphinx90Warning about str paths. docs/conf.py: prepend _ext to sys.path and rename the extension in the extensions list from sphinxcontrib.autodoc_doxygen to autodoc_doxygen. docs/conf.py also carries a monkey-patch for a parallel-build bug in upstream VACUMM/sphinx-fortran's FortranDomain.merge_domaindata. The upstream method references an undefined `outNames` (typo for `ourNames`) and also unpacks the wrong tuple shape from modules and objects dicts. With -j > 1 the merge silently fails and the f domain ends up empty, losing every Fortran cross-reference. The patch is heavily commented with a TODO to submit an upstream PR and drop the workaround during piece 2 of the upgrade. docs/requirements.txt: drop the sphinxcontrib-autodoc_doxygen git dependency. The other three forks remain pending pieces 2, 3, 4. Validation: built against a new venv.sph8 (Python 3.11 + Sphinx 8.2.3 + sphinx-rtd-theme 3.1.0 + sphinxcontrib-bibtex 2.6.5 + upstream VACUMM/sphinx-fortran master). `make html` produces 220 warnings vs 224 on the Sphinx 3 baseline. The f domain indexes 48 modules and 538 objects. Equation pages are effectively byte-identical (off only by Sphinx 8's section-id hoist optimization and the pilcrow-as-CSS theme change). Module pages are slightly larger because more cross-references now resolve than in the baseline build. * Swap sphinx-fortran fork for upstream VACUMM pinned commit Piece 2 of the sphinx toolchain upgrade. The jr3cermak/sphinx-fortran@1.2.2 fork existed to add Sphinx 3 API compatibility in 2020. Upstream VACUMM/sphinx-fortran has continued evolving since then (Sphinx logging API migration, parallel read support, modern setuptools, removal of the future dependency) and now works with Sphinx 8 directly. Upstream has not cut a PyPI release past 1.1.1 so we pin to a specific master commit for reproducibility. The pinned commit ships a broken FortranDomain.merge_domaindata that loses every f-domain object in parallel builds; the monkey-patch in conf.py from the previous commit handles that. We will revisit that patch and the pin together once an upstream PR is merged (see the TODO(piece-2) marker in conf.py setup). Validation: rebuilt docs with the pinned commit installed into venv.sph8. `make html` produces 220 warnings (identical to the pre-swap sph8 build), the f domain indexes 48 modules and 538 objects, mom.html has 36 internal cross-reference links, and f-modindex.html is generated. No behavior change from the sph8 build validated in the previous commit. * Drop sphinx fork for stock Sphinx 8 Piece 3 of the docs toolchain upgrade plan. This commit removes the last need for the patched sphinx fork and collapses the long list of transitive version ceilings it forced. Changes: - requirements.txt: drop git+https://github.com/jr3cermak/sphinx.git @v3.2.1mom6.4. Pin sphinx>=8,<9 from PyPI. Drop the eight ceiling lines (jinja2<3.1, sphinxcontrib_*<1.0.x, alabaster<0.7.14, setuptools<82.0.0) and the workaround comment around sphinx-fortran's broken requirements.txt; those existed only because Sphinx 3.2.1 dragged them in. Add lxml as an explicit dep (it is required by the vendored _ext/autodoc_doxygen extension to parse Doxygen XML). Keep six because the pinned sphinx-fortran commit imports it at module load time. - conf.py: add a monkey-patch for sphinx.util.math.wrap_displaymath that replicates the functional changes in the jr3cermak fork. The patch detects parts containing `\begin{equation}`, `\begin{eqnarray}`, or `\begin{align}` and emits them verbatim (no outer `\begin{equation}\begin{split}...\end{split} \end{equation}` wrapping). Without this, MOM6's math-heavy sources produce nested LaTeX environments that pdflatex chokes on. The patch is heavily commented with the rationale and a TODO marker for a possible upstream contribution. Verified by unit-testing the function directly (plain math still wrapped, explicit-environment math passed through verbatim) and by inspecting the generated _build/latex/ MOM6.tex: 0 double-wrapped equation/eqnarray/align blocks, 100 + 18 + 17 explicit environments preserved as top-level blocks. - conf.py: tighten exclude_patterns. Sphinx walks the entire source tree by default, which previously caused it to descend into local virtualenvs (`venv.sph3/`, `venv.sph8/`, `venv-3.11/`, etc.) and pick up LICENSE.rst, README.rst, and stray autosummary template files from inside site-packages. Each of those generated a "document isn't included in any toctree" warning and a corresponding stray .html file in the build output. The new exclusions cover venv*, venv-*, _build.*, and _ext/*/templates. Final build: 90 HTML files (down from 148, the lost 58 were all venv junk and template scaffolding) and 122 warnings (down from 220, with the same project- internal warnings as before plus the venv noise removed). - Makefile: add the equation post-processing hook to the html target, gated on UPDATEHTMLEQS=Y. The jr3cermak/sphinx fork carried this as a patch to sphinx/cmd/build.py so it ran after every sphinx-build invocation. Now invoked from the Makefile so it works against stock upstream Sphinx. The existing nortd target already had the same hook with different arguments (-p APIs -b doxygen vs -p html -b sphinx). The forked sphinx-fortran fix and the upstream PR for the math wrapping change are deferred to a follow-up. Validation: built from a fresh venv (venv.sph8.fresh) created by `python3.11 -m venv` and `pip install -r requirements.txt`. The install resolves cleanly with no manual intervention. `make html` succeeds with 122 warnings, indexes 48 modules and 538 f-domain objects, generates f-modindex.html, and produces 36 internal cross- reference links on mom.html. `make html UPDATEHTMLEQS=Y` runs the post-processing script successfully. `make latex` produces 0 double-wrapped math environments. * Drop flint dependency from docs build Piece 4 of the docs toolchain upgrade plan. With this commit the dependencies in docs/requirements.txt are only on maintained upstream sources. flint (marshallward/flint, vendored as jr3cermak/flint at 0.0.1) was historically pulled into the docs build with the intent of patching up Doxygen's incomplete parsing of Fortran functions with `result()` clauses. The fork's sphinxcontrib-autodoc_doxygen imported it at module load time but never actually called it; the import was a no-op behind a `try: import flint; except: pass`. We removed that import as part of piece 1's dead-code cleanup, so the vendored extension no longer touches flint at all. Verified empirically: uninstalled flint from venv.sph8.fresh and rebuilt. The build is byte-identical to the previous one with flint installed (122 warnings, 48 f-modules, 538 f-objects, 36 internal cross-reference links on mom.html, f-modindex.html present, 90 html files). Nothing in the current pipeline depends on flint. The two surviving "flint" mentions in docs/_ext/autodoc_doxygen/ are both in comments describing the original intent, not live code; they are left in place as historical context. If a future enhancement wants to actually fix Doxygen's result() clause parsing, that should be a separate piece of work and would not necessarily resurrect flint specifically. docs/README.md is updated to drop the flint bullet from the requirements list and the flint row from the credits table. Other stale entries in that section (sphinxcontrib_autodox-doxygen, the sphinx and sphinx-fortran fork rows) remain and will need a broader README sweep in a follow-up. * Sphinx upgrade follow-up fixes Consolidates four small fixes that followed the Sphinx 8 upgrade: - Switch all Makefile targets from `sphinx-build -b` to `-M` for consistent parallel build support. - Update docs/README.md to match the modernized toolchain (remove references to the four sphinx* forks, update requirements and credits sections). - Fix crash in autodoc_doxygen visit_image on empty <image> elements (node.text is None when doxygen produces <image/> without caption text). - Add __pycache__ and *.pyc to docs/.gitignore so the vendored extension's bytecode does not appear in git status. * RTD build infrastructure Consolidates four fixes for the Read the Docs build environment: - Override RTD's default sphinx runner with a build.jobs.build.html entry in .readthedocs.yml that runs sphinx-build -M html -j auto, since RTD's high-level sphinx: key has no parallelism option. - Make doxygen_xml path resolution robust to cwd changes: resolve relative paths against app.confdir rather than the ambient cwd, which differs between local builds and RTD. - Switch Makefile html target from -j 4 to -j auto to match RTD. - Enable html_static_path = ['_static'] in conf.py so custom CSS files (autodoxysource.css) are copied into the build output. Previously commented out, which meant app.add_css_file() added the <link> tag but the file was never deployed. * Fix O(N^2) XPath scans in autodoc_doxygen Two instances of the same class of bug in the vendored autodoc_doxygen extension, both scanning the entire merged doxygen XML tree (1230 files, 109 MB with programlisting) on every call: 1. scanNode used `node.xpath('//latexonly')` etc. In XPath, `//` starts from the document root, not from `node`. Since the extension merges all XML into one tree, every call scanned the whole tree. Fix: `//` -> `.//` (descendant-of-self). Recovery: 6m33s -> 48.8s wall clock at full input scale. 2. visit_ref used `get_doxygen_root().findall('.//*[@id=X]')` to resolve each prose <ref> by linearly scanning every element in the merged tree. With XML_PROGRAMLISTING=YES the tree tripled in size, making this the dominant cost (250s of 911s serial). Fix: lazy {id: element} dict built once on first use, O(1) lookup thereafter. Recovery: 911s -> 676s serial. * Add source code browser to Sphinx docs Generate one HTML page per Fortran source file with syntax highlighting and clickable cross-references. Clicking an identifier in the source jumps to its API documentation page; each API entry gets a [source] link back to the source listing. Implementation: - Enable XML_PROGRAMLISTING in Doxyfile_rtd (standalone commit originally, now folded in). - New autodoxysource directive in _ext/autodoc_doxygen/ that walks doxygen's <programlisting> XML, emitting per-line anchors, CSS-classed highlight spans, and pending_xref nodes for identifiers. - Stub generator produces 329 :orphan: source pages under api/generated/source/. - [source] links added to DoxygenMethodDocumenter, DoxygenTypeDocumenter, and DoxygenModuleDocumenter. - CSS styled to match the sphinx_rtd_theme's code blocks (font stack, size, colors, line gutter). - Node count optimization: coalesce text runs in _walk_highlight (10-30 nodes/line -> 3-5), set support_smartquotes=False on the source container to skip the smartquotes transform. Combined with the visit_ref fix, total build time with the source browser is 115s parallel (vs 360s before optimization). * Improve API documentation Consolidates five improvements to the rendered API pages: - Show Fortran type declarations for function/subroutine parameters and derived-type members. Types are rendered as inline code (e.g. ``real, intent(in), optional``) by looking up the <param> elements on the parent <memberdef>. - Move [source] links from the top of each entry to the bottom, so the description and parameters appear first. - Fix functions (as opposed to subroutines) not being registered in the sphinx-fortran domain. format_name() was prepending the return type (e.g. "real") which sphinx-fortran's f_sig_re regex cannot parse, leaving function entries without anchors. - Add Functions and Source Files index pages to the API Reference, with cross-reference links to module pages and source browser pages respectively. - Exclude upstream CVMix sources from doxygen input to avoid warnings from undocumented third-party code. * Fix multi-line math blocks losing indentation in LaTeX Multi-line math content (e.g. \begin{align}...\end{align}) from doxygen XML was emitted as: .. math:: \begin{align} \mathbf{v} = \mathbf{u} + ... \end{align} RST requires directive body content to be indented. The continuation lines at column 0 were parsed as regular text by docutils, producing garbled LaTeX output with "Runaway argument" and "Missing $" errors — 164 LaTeX errors on the Notation page alone. Fix: in visit_formula, detect multi-line math text and emit it as a properly indented block: .. math:: \begin{align} \mathbf{v} = \mathbf{u} + ... \end{align} Single-line math is unchanged (stays on the .. math:: line). After fix: `make latexpdf` LaTeX errors drop from 164 to 10. The remaining 10 are unrelated (9 Unicode Greek characters in source comments that pdflatex cannot render, 1 duplicate label).
OlgaSergienko
added a commit
to OlgaSergienko/MOM6
that referenced
this pull request
Apr 28, 2026
* Fixes for flang * Change variable not type * Refactor update_OBC_segment_data This commit refactors normal velocity/transport calculation in update_OBC_segment_data, using the orientation-agnostic loop ranges. Note that there is still an orientation-branch because the grid spacing G%dyCu and G%dxCv are different. * The field segment%h is removed, as we can just use h directly. * Comments are added to better illustrate the subroutine structure * Suppress OBC unnecessary field warning with T/S Remove inaccurate warning message in logs when OBC_TEMP_SALT_NEEDED_BUG is false but temperature and salinity are needed and provided. * Bugfix for OBC using VALUE mode This commit fixes the bug that OBC segment data does not update if only value mode is used for all segments. Previously, there were two issues, 1. Subroutine update_OBC_data, which is a wrapper over subroutine update_OBC_segment_data is only called if global flag OBC%update_OBC is True. This flag, however, is only turned on if there OBC is specified by files. 2. Inside of update_OBC_data, subroutine update_OBC_segment_data is only called if global flags OBC%any_needs_IO_for_data or OBC%add_tide_constituents is True, which again prohibits OBC update with "value". Example: if normal velocity is specified by value, the transport is calculate once during initialization using initial thickness. And the transport is never updated through the run. And the model will restart with a different transport. A new runtime parameter OBC_VALUE_UPDATE_BUG is added to fix/recover this bug. In addition, OBC%update_OBC is set to True if initialize_segment_data is called. a separate procedure is added to set any_needs_IO_for_data, for clarity. * +Diag_mediator enhancements from ice shelf version Added diag_mediator capabilities that had been developed and used in the SIS2 and MOM_ice_shelf versions of the diag mediator, in preparation for some of these separate versions to be combined into a single version. This includes adding an overload to the register_scalar_field() interface so that scalars can be registered with or without providing a null axis, retaining the existing interface while adding the one that is widely used with the ice shelf code. The previous routine register_scalar_field() is now register_scalar_field_CS(), but it has the same public interface. This commit also adds the new public interface MOM_diag_send_complete() that calls the FMS routine that flushes the IO buffers. Although this call can be too expensive for production runs, it can be very useful for debugging. Internally, register_diag_field() now checks for standard axes for 2-d fields, analogously to what was previously done for 3-d fields, thereby avoiding the allocation of a separate axis group for each 2-d diagnostic. This has been the case for ice-shelf code for some time, and it will save memory will giving identical results. The testing for incompatible mask and array sizes was consolidated into a single call each in post_data_2d_low() and post_data_3d_low(), paving the way for adding the option of masking static fields. All solutions are bitwise identical and diagnostic output (including metadata) is identical to what was previously output, but there are two new public interfaces. * +Standardize q-point index space axis names Renamed the q-point index space axis names to be uniformly Iq and Jq, rather than having names that change depending on whether or not symmetric memory is being used. This commit also changes the long names of the grid space axes to be consistent between the x and y axes and to mirror those that have been adopted in the ice shelf code. The order of the logical tests for index space axes and symmetric memory around the calls to diag_axis_init were swapped to that corresponding cases are next to each other, perhaps making any inconsistencies more obvious. There were also minor changes to the code setting the axis label variables to follow the MOM6 case convention for indices and the spacing conventions for assignments, as described in the MOM6 style guide. All solutions are bitwise identical, but there are changes to the names of the axis label variables and their long descriptions when USE_INDEX_DIAGNOSTIC_AXES is true. * *Fix tracer content tendency cell_methods Corrected the cell_methods for 8 families of tracer content tendency diagnostics. Without this change, the 2-fold downscaled tendencies are 4 times too large, and the 3-fold downscaled tendencies 9 times too large. The misleading comment describing the local `conv_units` string variable in register_tracer_diagnostics() was also fixed. All solutions and standard diagnostics are bitwise identical, but there are changes in the values of downscaled diagnostics and changes in metadata. * makedep: support empty macros and defines Makedep preprocessor parsing assumed define macros of the form `#define macro(X) some-macro` but did not support empty macros of the form `#define macro(X)`. Preprocessor flag macros were assumed to be of the form `-DMACRO=VALUE`, and did not support empty macros `-DMACRO`. This patch addresses both issues. * Updates for layout reproducing and restart reproducing with remap_aux_vars - To reproduce across restart files requires an additional halo update on diffu and diffv in MOM_dynamics_split_RK2.F90 - To reproduce across layouts with vertex shear option requires a halo update on thickness before remapping the viscosities on the vertex points. * Scale tideamp to L/T instead of Z/T - Setting BBL_USE_TIDAL_BG = True causes the model to fail dimensional consistency test. If you set the spatial scale as m_to_L instead of m_to_Z in MOM_set_viscosity it fixes the issue. - Fixing documentation fo L/T scaling of tideamp - Change suggested Hallberg * Adding metric terms to account for non-rectangular shape of grid cells (mom-ocean#1079) This PR corrects an implementation of a finite-element solver for ice-sheet/shelf velocity. It computes a Jacobian weight J_q in shape functions for quadrature points and applies it to integrals over cells. This correction is significant when the shape of the cell is not rectangular. The same corrections are applied for subgrid parameterizations. Additionally, it fixes missing deallocations of several arrays. Changes made: - nitialize_ice_shelf_dyn: allocates a new variable `Jac` - bilinear_shape_fn_grid: computes a Jacobian determinant (`Jac`) at each quadrature point - CG_action: uses `jac_wt= Jac * IareaT` for computing the ice deformation and basal traction terms - matrix_diagonal: uses `jac_wt= Jac * IareaT` for computing diagonal terms - CG_action_subgrid_basal: computes `jac_sub_wt` using new arguments `dxCv_S`, `dxCv_N`,`dyCv_E` and `dyCv_W` that determine cell-edge spacing - ice_shelf_dyn_end: deallocates `Jac`, `CS%sx_shelf`, `CS%sy_shelf`, `CS%u_flux_bndry_val`, `CS%v_flux_bndry_val`,`CS%calv_mask`, `CS%Phi`, `CS%Phisub`, `CS%PhiC`. Copilot (model Claude Sonet 4.6) assisted with this PR. * Modernize Sphinx docs toolchain, add RTD build, and expand API docs (mom-ocean#1089) * Vendor sphinxcontrib-autodoc_doxygen into docs/_ext/ This is piece 1 of the sphinx toolchain upgrade. The docs build previously depended on four forks which haven't been updated since 2020. The chain pinned the entire build to Sphinx 3.2.1, requiring a growing list of transitive version ceilings (jinja2<3.1, sphinxcontrib_*<1.0.x, alabaster<0.7.14, setuptools<82). This commit handles the sphinxcontrib-autodoc_doxygen dependency. The upstream (rmcgibbo) has been dormant since June 2021; the fork was effectively a rewrite (~90% of xmlutils.py changed, both documenters replaced, the whole domain switched from cpp to f). Monkey-patching the upstream package was rejected because there was nothing stable to patch against. Instead the fork's code is vendored in-tree at docs/_ext/autodoc_doxygen/ so it can be debugged, blamed, and edited like any other project source. Changes vs the fork at tag 0.7.13: - Renamed doxynamespace.rst template to doxymodule.rst and dropped the unused C++ doxyclass.rst (DoxygenClassDocumenter was already unregistered in the fork). - Removed ~80 lines of dead code: commented-out pdb.set_trace lines, the unregistered DoxygenClassDocumenter class, the dead visit_ref_angus alternative, _import_by_name_original, and the unused try-import of flint. - Dropped Python 2 compatibility cruft: from __future__ imports, from six import itervalues (replaced with dict.values()). - Ported four Sphinx 3 -> 8 API changes: * DoxygenAutosummary.get_items now passes self.bridge (the DocumenterBridge) to documenter constructors instead of self, so Documenter.__init__ finds directive.genopt. * self.warn / self.directive.warn calls (removed in Sphinx 8) replaced with sphinx.util.logging.getLogger(...).warning. * Wrapped env.doc2path in os.fspath to silence the RemovedInSphinx90Warning about str paths. docs/conf.py: prepend _ext to sys.path and rename the extension in the extensions list from sphinxcontrib.autodoc_doxygen to autodoc_doxygen. docs/conf.py also carries a monkey-patch for a parallel-build bug in upstream VACUMM/sphinx-fortran's FortranDomain.merge_domaindata. The upstream method references an undefined `outNames` (typo for `ourNames`) and also unpacks the wrong tuple shape from modules and objects dicts. With -j > 1 the merge silently fails and the f domain ends up empty, losing every Fortran cross-reference. The patch is heavily commented with a TODO to submit an upstream PR and drop the workaround during piece 2 of the upgrade. docs/requirements.txt: drop the sphinxcontrib-autodoc_doxygen git dependency. The other three forks remain pending pieces 2, 3, 4. Validation: built against a new venv.sph8 (Python 3.11 + Sphinx 8.2.3 + sphinx-rtd-theme 3.1.0 + sphinxcontrib-bibtex 2.6.5 + upstream VACUMM/sphinx-fortran master). `make html` produces 220 warnings vs 224 on the Sphinx 3 baseline. The f domain indexes 48 modules and 538 objects. Equation pages are effectively byte-identical (off only by Sphinx 8's section-id hoist optimization and the pilcrow-as-CSS theme change). Module pages are slightly larger because more cross-references now resolve than in the baseline build. * Swap sphinx-fortran fork for upstream VACUMM pinned commit Piece 2 of the sphinx toolchain upgrade. The jr3cermak/sphinx-fortran@1.2.2 fork existed to add Sphinx 3 API compatibility in 2020. Upstream VACUMM/sphinx-fortran has continued evolving since then (Sphinx logging API migration, parallel read support, modern setuptools, removal of the future dependency) and now works with Sphinx 8 directly. Upstream has not cut a PyPI release past 1.1.1 so we pin to a specific master commit for reproducibility. The pinned commit ships a broken FortranDomain.merge_domaindata that loses every f-domain object in parallel builds; the monkey-patch in conf.py from the previous commit handles that. We will revisit that patch and the pin together once an upstream PR is merged (see the TODO(piece-2) marker in conf.py setup). Validation: rebuilt docs with the pinned commit installed into venv.sph8. `make html` produces 220 warnings (identical to the pre-swap sph8 build), the f domain indexes 48 modules and 538 objects, mom.html has 36 internal cross-reference links, and f-modindex.html is generated. No behavior change from the sph8 build validated in the previous commit. * Drop sphinx fork for stock Sphinx 8 Piece 3 of the docs toolchain upgrade plan. This commit removes the last need for the patched sphinx fork and collapses the long list of transitive version ceilings it forced. Changes: - requirements.txt: drop git+https://github.com/jr3cermak/sphinx.git @v3.2.1mom6.4. Pin sphinx>=8,<9 from PyPI. Drop the eight ceiling lines (jinja2<3.1, sphinxcontrib_*<1.0.x, alabaster<0.7.14, setuptools<82.0.0) and the workaround comment around sphinx-fortran's broken requirements.txt; those existed only because Sphinx 3.2.1 dragged them in. Add lxml as an explicit dep (it is required by the vendored _ext/autodoc_doxygen extension to parse Doxygen XML). Keep six because the pinned sphinx-fortran commit imports it at module load time. - conf.py: add a monkey-patch for sphinx.util.math.wrap_displaymath that replicates the functional changes in the jr3cermak fork. The patch detects parts containing `\begin{equation}`, `\begin{eqnarray}`, or `\begin{align}` and emits them verbatim (no outer `\begin{equation}\begin{split}...\end{split} \end{equation}` wrapping). Without this, MOM6's math-heavy sources produce nested LaTeX environments that pdflatex chokes on. The patch is heavily commented with the rationale and a TODO marker for a possible upstream contribution. Verified by unit-testing the function directly (plain math still wrapped, explicit-environment math passed through verbatim) and by inspecting the generated _build/latex/ MOM6.tex: 0 double-wrapped equation/eqnarray/align blocks, 100 + 18 + 17 explicit environments preserved as top-level blocks. - conf.py: tighten exclude_patterns. Sphinx walks the entire source tree by default, which previously caused it to descend into local virtualenvs (`venv.sph3/`, `venv.sph8/`, `venv-3.11/`, etc.) and pick up LICENSE.rst, README.rst, and stray autosummary template files from inside site-packages. Each of those generated a "document isn't included in any toctree" warning and a corresponding stray .html file in the build output. The new exclusions cover venv*, venv-*, _build.*, and _ext/*/templates. Final build: 90 HTML files (down from 148, the lost 58 were all venv junk and template scaffolding) and 122 warnings (down from 220, with the same project- internal warnings as before plus the venv noise removed). - Makefile: add the equation post-processing hook to the html target, gated on UPDATEHTMLEQS=Y. The jr3cermak/sphinx fork carried this as a patch to sphinx/cmd/build.py so it ran after every sphinx-build invocation. Now invoked from the Makefile so it works against stock upstream Sphinx. The existing nortd target already had the same hook with different arguments (-p APIs -b doxygen vs -p html -b sphinx). The forked sphinx-fortran fix and the upstream PR for the math wrapping change are deferred to a follow-up. Validation: built from a fresh venv (venv.sph8.fresh) created by `python3.11 -m venv` and `pip install -r requirements.txt`. The install resolves cleanly with no manual intervention. `make html` succeeds with 122 warnings, indexes 48 modules and 538 f-domain objects, generates f-modindex.html, and produces 36 internal cross- reference links on mom.html. `make html UPDATEHTMLEQS=Y` runs the post-processing script successfully. `make latex` produces 0 double-wrapped math environments. * Drop flint dependency from docs build Piece 4 of the docs toolchain upgrade plan. With this commit the dependencies in docs/requirements.txt are only on maintained upstream sources. flint (marshallward/flint, vendored as jr3cermak/flint at 0.0.1) was historically pulled into the docs build with the intent of patching up Doxygen's incomplete parsing of Fortran functions with `result()` clauses. The fork's sphinxcontrib-autodoc_doxygen imported it at module load time but never actually called it; the import was a no-op behind a `try: import flint; except: pass`. We removed that import as part of piece 1's dead-code cleanup, so the vendored extension no longer touches flint at all. Verified empirically: uninstalled flint from venv.sph8.fresh and rebuilt. The build is byte-identical to the previous one with flint installed (122 warnings, 48 f-modules, 538 f-objects, 36 internal cross-reference links on mom.html, f-modindex.html present, 90 html files). Nothing in the current pipeline depends on flint. The two surviving "flint" mentions in docs/_ext/autodoc_doxygen/ are both in comments describing the original intent, not live code; they are left in place as historical context. If a future enhancement wants to actually fix Doxygen's result() clause parsing, that should be a separate piece of work and would not necessarily resurrect flint specifically. docs/README.md is updated to drop the flint bullet from the requirements list and the flint row from the credits table. Other stale entries in that section (sphinxcontrib_autodox-doxygen, the sphinx and sphinx-fortran fork rows) remain and will need a broader README sweep in a follow-up. * Sphinx upgrade follow-up fixes Consolidates four small fixes that followed the Sphinx 8 upgrade: - Switch all Makefile targets from `sphinx-build -b` to `-M` for consistent parallel build support. - Update docs/README.md to match the modernized toolchain (remove references to the four sphinx* forks, update requirements and credits sections). - Fix crash in autodoc_doxygen visit_image on empty <image> elements (node.text is None when doxygen produces <image/> without caption text). - Add __pycache__ and *.pyc to docs/.gitignore so the vendored extension's bytecode does not appear in git status. * RTD build infrastructure Consolidates four fixes for the Read the Docs build environment: - Override RTD's default sphinx runner with a build.jobs.build.html entry in .readthedocs.yml that runs sphinx-build -M html -j auto, since RTD's high-level sphinx: key has no parallelism option. - Make doxygen_xml path resolution robust to cwd changes: resolve relative paths against app.confdir rather than the ambient cwd, which differs between local builds and RTD. - Switch Makefile html target from -j 4 to -j auto to match RTD. - Enable html_static_path = ['_static'] in conf.py so custom CSS files (autodoxysource.css) are copied into the build output. Previously commented out, which meant app.add_css_file() added the <link> tag but the file was never deployed. * Fix O(N^2) XPath scans in autodoc_doxygen Two instances of the same class of bug in the vendored autodoc_doxygen extension, both scanning the entire merged doxygen XML tree (1230 files, 109 MB with programlisting) on every call: 1. scanNode used `node.xpath('//latexonly')` etc. In XPath, `//` starts from the document root, not from `node`. Since the extension merges all XML into one tree, every call scanned the whole tree. Fix: `//` -> `.//` (descendant-of-self). Recovery: 6m33s -> 48.8s wall clock at full input scale. 2. visit_ref used `get_doxygen_root().findall('.//*[@id=X]')` to resolve each prose <ref> by linearly scanning every element in the merged tree. With XML_PROGRAMLISTING=YES the tree tripled in size, making this the dominant cost (250s of 911s serial). Fix: lazy {id: element} dict built once on first use, O(1) lookup thereafter. Recovery: 911s -> 676s serial. * Add source code browser to Sphinx docs Generate one HTML page per Fortran source file with syntax highlighting and clickable cross-references. Clicking an identifier in the source jumps to its API documentation page; each API entry gets a [source] link back to the source listing. Implementation: - Enable XML_PROGRAMLISTING in Doxyfile_rtd (standalone commit originally, now folded in). - New autodoxysource directive in _ext/autodoc_doxygen/ that walks doxygen's <programlisting> XML, emitting per-line anchors, CSS-classed highlight spans, and pending_xref nodes for identifiers. - Stub generator produces 329 :orphan: source pages under api/generated/source/. - [source] links added to DoxygenMethodDocumenter, DoxygenTypeDocumenter, and DoxygenModuleDocumenter. - CSS styled to match the sphinx_rtd_theme's code blocks (font stack, size, colors, line gutter). - Node count optimization: coalesce text runs in _walk_highlight (10-30 nodes/line -> 3-5), set support_smartquotes=False on the source container to skip the smartquotes transform. Combined with the visit_ref fix, total build time with the source browser is 115s parallel (vs 360s before optimization). * Improve API documentation Consolidates five improvements to the rendered API pages: - Show Fortran type declarations for function/subroutine parameters and derived-type members. Types are rendered as inline code (e.g. ``real, intent(in), optional``) by looking up the <param> elements on the parent <memberdef>. - Move [source] links from the top of each entry to the bottom, so the description and parameters appear first. - Fix functions (as opposed to subroutines) not being registered in the sphinx-fortran domain. format_name() was prepending the return type (e.g. "real") which sphinx-fortran's f_sig_re regex cannot parse, leaving function entries without anchors. - Add Functions and Source Files index pages to the API Reference, with cross-reference links to module pages and source browser pages respectively. - Exclude upstream CVMix sources from doxygen input to avoid warnings from undocumented third-party code. * Fix multi-line math blocks losing indentation in LaTeX Multi-line math content (e.g. \begin{align}...\end{align}) from doxygen XML was emitted as: .. math:: \begin{align} \mathbf{v} = \mathbf{u} + ... \end{align} RST requires directive body content to be indented. The continuation lines at column 0 were parsed as regular text by docutils, producing garbled LaTeX output with "Runaway argument" and "Missing $" errors — 164 LaTeX errors on the Notation page alone. Fix: in visit_formula, detect multi-line math text and emit it as a properly indented block: .. math:: \begin{align} \mathbf{v} = \mathbf{u} + ... \end{align} Single-line math is unchanged (stays on the .. math:: line). After fix: `make latexpdf` LaTeX errors drop from 164 to 10. The remaining 10 are unrelated (9 Unicode Greek characters in source comments that pdflatex cannot render, 1 duplicate label). * Refactor Recon1d reconstructions and fix PLM_WLS LS_error Under optimization -O2 with ifort we had some floating-point divergences in the new Recon1d schemes. Two schemes diverged from their counterparts that use the old "select" pathways. One scheme failed self-consistency tests due to inlining allowing some aggressive optimization to occur. Recon1d_PPM_H4_2019: - Add explicit parentheses in end_value_h4() to enforce evaluation order and ensure bit-reproducibility across compilers. Recon1d_PPM_hybgen: - Refactor reconstruct() into a cleaner two-pass approach using new private helpers (hybgen_ppm_coefs, bound_edge_values, check_discontinuous_edge_values) copied from phased-out modules to avoid external dependencies. Bit-for-bit with the old PPM_HYBGEN scheme. - Add average() override that clamps sub-cell averages to [min(ul,ur), max(ul,ur)], reproducing force_bounds_in_subcell behaviour. Recon1d_PLM_WLS: - Fix LS_error(): remove 0.5/0.25 factors and reformulate as squared deviation from the optimal slope. - Also updated Doxygen math which had some errors. - In check_reconstruction(), copy state via assignment (perturbed = this) since an inline call to %reconstruct() on same data was yielding LSB differences. --------- Co-authored-by: Matthew Thompson <matthew.thompson@nasa.gov> Co-authored-by: Marshall Ward <marshall.ward@noaa.gov> Co-authored-by: He Wang <He.Wang@noaa.gov> Co-authored-by: Robert Hallberg <Robert.Hallberg@noaa.gov> Co-authored-by: brandon.reichl <brandon.reichl@noaa.gov> Co-authored-by: Alistair Adcroft <adcroft@users.noreply.github.com> Co-authored-by: Alistair Adcroft <Alistair.Adcroft@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
4 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.
This PR includes a set of changes that introduce rescaling of pressures and
related variables for dimensional consistency testing. These changes include
extensive extensions to the interfaces for the equation of state and the
introduction of new arguments to several routines. The new interfaces are
widely used throughout the MOM6 code. There are no changes to solutions,
diagnostics, or output files, but answers could change in cases that were trying
to use ALE remapping and a RHO coordinate with a reference pressure other than
the previously hard-coded value of 2000 dbar, as the reference pressure will now
be properly set at run-time. The commits in this PR include: