Latest dev/gfdl#13
Merged
Merged
Conversation
Resolves merge conflict due to new PLM_WLS test.
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
Remove inaccurate warning message in logs when OBC_TEMP_SALT_NEEDED_BUG is false but temperature and salinity are needed and provided.
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.
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.
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.
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 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.
…_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.
- 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
#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.
…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).
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.
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
6 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.
No description provided.