Skip to content

docs: bring the documentation in line with the code - #488

Merged
sameeul merged 23 commits into
PolusAI:mainfrom
darkclad:main-docs-update
Oct 22, 2026
Merged

sameeul merged 23 commits into
PolusAI:mainfrom
darkclad:main-docs-update

Conversation

@darkclad

@darkclad darkclad commented Oct 15, 2026 •

Copy link
Copy Markdown

Summary

This branch brings the Nyxus documentation back in line with the code. The site is reorganized into a user guide, a feature reference, an API reference and a developer guide. Every feature family has a page in one template, each page's numbers are checked by a script that the test suite runs, and the feature list and option reference are generated from the code. The README shrinks to a landing page that links into the site.

Sixteen commits, one per area (build, entry pages, command line, input and output, scale and GPU, guide pages, feature catalogue, example tests, six groups of feature pages, API docstrings, developer guide), so the series can be reviewed commit by commit. Two commits then fix overflow examples on arm64/macOS, and three address the first review: the checks and CI, the pages, scripts and registry rows, and the issue links. Only the last commit is meant to build cleanly on its own.

Changes

Build and structure

  • The docs build without the compiled extension, and CI fails a PR on any Sphinx warning. A separate job checks the external links with retries and reports broken ones without failing the workflow, since external sites rate-limit CI runners.
  • The site uses pydata-sphinx-theme with four sections and a card landing page with a citation block.
  • The feature list, the feature groups and the command-line reference are generated from the code (docs/tools/gen_feature_reference.py, docs/tools/gen_cli_reference.py, each with a --check mode). The feature list shows each feature's validation status from tests/vetting/oracle_coverage.csv. The CLI reference finds each default by the variable or call it is assigned to (default_at = "environment.cpp:n_pixel_distance"), so moving code within a file does not fail the check and changing a default does.

Feature pages (docs/source/Math/), one template for every family: how the values are computed, a worked example, notation, settings with their real effect, output columns, a generated index with IBSI identifiers and validation status, one entry per feature with the formula as the code computes it, possible issues with measured values, validation and references.

User guide

  • Installation, with the CMake options table.
  • Command line and Python examples, and supported formats.
  • 3D volumes, and whole images and large images (out of core).
  • Output formats and columns, GPU acceleration and IBSI mode.
  • Image quality, the WIPP plugin, and troubleshooting and performance.
  • Concept pages on how features are validated and on the conventions every family shares: the ROI, grey-level binning, 2D and 3D defaults, background, intensity domain, units and anisotropy, constant ROIs and the entropy logarithm.
  • A release notes page linking the notes of each tagged release.

Tested examples

  • Every Python example on the site is a script in docs/examples/, included with literalinclude. tests/python/test_docs_examples_mechanics.py runs each script in a fresh directory.
  • Each worked-example script asserts the values its page quotes, the index values against docs/feature_extras.toml, which also feeds the index tables. A script's header names the values it does not assert: results of a 32-bit overflow on main, and values only the command line can produce (a negative or zero grey depth).

API reference

  • The nyxus module's docstrings are rewritten in numpydoc style and corrected against the code: defaults, group names, case-sensitivity, parameters that do not exist, and return values.
  • Apart from docstrings, one user-visible text changes: the ValueError that Nyxus, Nyxus3D and ImageQuality raise for a coarse_gray_depth below 1 now says "must be positive" instead of "must be non-negative", since 0 is refused too. test_api_arguments_mechanics.py asserts the new text. Everything else in nyxus.py and functions.py is unchanged: their docstring-stripped syntax trees differ from the previous ones only in those three strings.

Developer and project files

  • The developer guide is rewritten for the current architecture.
  • New: CONTRIBUTING.md and CITATION.cff. release-please now updates the version in CITATION.cff.
  • The contributor notes and the vetting README, SPEC, MIGRATION and TOOLS are corrected where they disagreed with the code; every build recipe uses build-cli and an explicit Release build.
  • plugin.json (the shipped WIPP manifest) offers the arrowipc output type that the program accepts, in place of arrow; marks intDir and segDir as required, since every run needs both; and drops about 50 feature options that the program does not accept (*ALL_BUT_GABOR* and *ALL_BUT_GLCM*, which define no features, and names such as EQUIVALENT_DIAMETER and TEXTURE_* that are not feature codes).
  • Unused images are removed, and the pipeline diagram is redrawn to match the code (it showed an online mode no code path uses, and only CSV and pandas output).
  • The Hounsfield and OME-Zarr fixture READMEs under tests/data are corrected.

Policy change in the contributor notes

  • CLAUDE.md and CONTRIBUTING.md change one rule rather than correct it: a defect fix no longer gets a source comment describing the defect and the fix. Comments describe what the code does now, and the history of a change (the defect and the fix) goes in the commit message and the PR description. Maintainers may prefer to decide this separately; the two paragraphs can be dropped from this PR without affecting anything else.

README

  • Cut to a description, installation, three quick starts that run as written, links into the site, the citation and the license. Links are absolute, so the PyPI page renders them.

Test plan

  • python -m sphinx -b html -W --keep-going docs/source docs/build/html: passes, 0 warnings.
  • python -m sphinx -b linkcheck docs/source docs/build/linkcheck: passes.
  • python docs/tools/gen_feature_reference.py --check (now also checks the feature counts in README.md and index.rst) and python docs/tools/gen_cli_reference.py --check: pass.
  • pytest tests/python/test_docs_examples_mechanics.py: 48 passed (every example and worked-example script, and the README quick starts).
  • pytest tests/python: 275 passed, 4 skipped on Python 3.12, and 259 passed, 5 skipped on Python 3.10 (the docs examples avoid tomllib, so they run on 3.10 too), including the docstring examples and the docs generator and configuration checks.
  • tests/vetting/check_coverage.py --check, tests/vetting/check_test_names.py and tests/python/test_vetting_mechanics.py: pass.
  • Docstring pass: the docstring-stripped syntax trees of nyxus.py and functions.py differ from those before the change only in the three ValueError strings named above.
  • Every worked example was reproduced by an independent implementation (numpy, scipy, scikit-image, PyRadiomics or MIRP), and each script fails when one quoted value is changed.

Demian Vladi added 16 commits October 14, 2026 18:04
…I on warnings

conf.py imports the Python package from the source tree and mocks nyxus.backend, so the docs build
without compiling Nyxus. The site uses pydata-sphinx-theme; the page title carries no version
number, and |release| (quoted by the citation) is read from CITATION.cff, which release-please now
updates. A docs workflow builds the site with warnings as errors and checks links. The docs
dependencies move from environment.yml to docs/requirements.txt, which the Read the Docs
configuration uses.

test_docs_conf_mechanics.py checks the release, the title and that |release| is still used.
The README becomes a landing page: what Nyxus is, how to install it, three quick starts that run as
written, links into the site, the citation and the license, with absolute links so the PyPI page
renders them. The site gets a card landing page with the citation, an installation page (packages,
platforms, GPU packages, building from source with the CMake options), the user and developer guide
section pages, and a release notes page. Package facts describe what is published now; release
gaps on the PyPI and conda-forge builds are stated where they matter.
…pages

docs/tools/gen_cli_reference.py renders docs/source/cli_reference.rst from docs/cli_options.toml,
where each option records its default, the source line that assigns it, its values, its effect and
the code it was checked against. Its --check mode fails when the TOML and the option header
disagree, a default does not match its source line, or a citation is stale
(test_docs_cli_reference_mechanics.py). The command-line and examples page and the WIPP page are
rewritten around examples that run; plugin.json offers the arrowipc output type the program
accepts.
…onventions

Supported formats lists what each reader takes (TIFF, OME-TIFF, OME-Zarr, NIfTI, DICOM), how
intensities become grey levels and which builds include which reader. The output page describes
the columns, CSV, Arrow IPC and Parquet, and missing values. The conventions page collects what
every family shares: the ROI and its bounding box, grey-level binning, 2D and 3D defaults,
background, the intensity domain, units and anisotropy, constant ROIs and the entropy logarithm.
The large-images page explains the RAM limit and out-of-core processing: which families match the
in-memory values on an image read as one tile, which raise an error or return wrong values on an
image read as several tiles, and which inputs are refused. The volumetric page covers 3D input,
anisotropy and physical spacing, whole-volume mode and 3D streaming. The GPU page lists the
families computed on the GPU, how the GPU is selected from Python and the command line, and where
GPU values differ from the CPU.
The IBSI mode page says what --ibsi changes, family by family. The image quality page describes
the ImageQuality class and its group. The validation page explains how features are checked
against independent implementations and what the coverage registry records. The troubleshooting
page quotes each common error as Nyxus prints it, from Python and the command line, with its cause,
and covers performance.
…indexes

docs/tools/gen_feature_reference.py reads the feature registry from the code
(docs/tools/feature_registry.py) and renders the feature list, the feature groups and one index
per family from docs/feature_descriptions.toml (a one-line description of every code) and
docs/feature_extras.toml (names, IBSI identifiers and worked-example values). The feature list
shows each feature's validation status from tests/vetting/oracle_coverage.csv. Its --check mode
fails on a missing or empty description, an unclaimed feature class, a stale generated file or a
page whose feature counts are out of date (test_docs_feature_reference_mechanics.py).
test_2d_docs_examples_mechanics.py runs each script in docs/examples in a fresh directory and
fails on a non-zero exit. Worked-example scripts assert every value their page quotes against
docs/feature_extras.toml, which the harness hands them as JSON, so the scripts run on Python 3.10
without tomllib. check_test_names.py registers the dimension-agnostic docs tests.
… pages

Each page follows the family template: how the values are computed, a worked example whose values
a script checks, notation, settings with their real effect, output columns, the generated index,
one entry per feature with the formula as the code computes it, possible issues with measured
values, validation and references. The IBSI intensity histogram page is new. Known defects are
stated as current behaviour with "a fix is coming".
…l pages

The morphology page covers its families (basic morphology, contour, convex hull, ellipse,
circles, Feret, Martin, Nassenstein, chords, Euler, extrema, geodetic, ROI radius, neighbours) in
the family template; erosion, fractal dimension and the radial distribution (new) get their own
pages, each with a checked worked example. Images the new pages no longer use are removed. The
comments of the scikit-image erosion oracle and its generator now describe the structuring element
and the counting convention Nyxus uses.
The shape and intensity moments, the Zernike moments and the Gabor features get pages in the
family template with checked worked examples. The Gabor figure is regenerated by
docs/tools/gen_gabor_figure.py from the parameters the code uses.
The three 2D texture families get pages in the family template, with checked worked examples,
their settings (angles, offset, grey depth, IBSI mode) and their differences from IBSI and
PyRadiomics measured.
The four remaining 2D texture families get pages in the family template, with checked worked
examples, their neighbourhood and zone definitions, and their differences from IBSI and
PyRadiomics measured.
New pages for the 3D intensity and shape families and the seven 3D texture families (GLCM, GLRLM,
GLSZM, GLDM, GLDZM, NGLDM, NGTDM), in the family template with checked worked examples: how each
differs from its 2D counterpart, which settings and metaparameters apply, anisotropy and
out-of-core behaviour, and validation against MIRP, PyRadiomics and IBSI.
…them against the code

The docstrings of the nyxus module are rewritten in numpydoc style and corrected against the code:
defaults, accepted values, group names, case sensitivity, the memory limit, anisotropy, GPU
selection, Raises sections and return values. The API reference pages render them.
test_docstring_examples_mechanics.py runs every docstring example and
test_api_arguments_mechanics.py checks the documented argument errors.

Docstring-only: the docstring-stripped syntax trees of nyxus.py and functions.py are unchanged.
…otes

The developer guide is rewritten for the current architecture: the run flow, adding a feature or
an option, settings and metaparameters, the dispatch to in-memory, out-of-core and GPU paths,
testing and vetting, CI, the documentation tooling and releases. CONTRIBUTING.md is new; the
contributor notes and the vetting README, SPEC, MIGRATION and TOOLS are corrected where they
disagreed with the code, and every build recipe uses build-cli with an explicit Release build. The
pipeline diagram is redrawn to match the code, and a coverage-registry row and the fixture READMEs
are corrected.
Demian Vladi added 2 commits October 15, 2026 07:07
…mples

Two worked examples quote the effect of an overflow that is undefined behaviour in C++, and the
arm64 and macOS CI builds give another result than x86-64:

- fractal dimension: an intensity of 4294967295 plus 1, converted from double to a 32-bit
  unsigned integer, is 0 on x86-64 (the pixel is dropped from the box count) and saturates on ARM
  (the pixel is counted). The script expects the x86-64 value only on x86-64.
- GLSZM large area emphasis on a 250 x 250 zone: the 32-bit signed square wraps on most builds,
  and one macOS ARM build returned the true value. The script accepts either.

Both pages now say that the value depends on the build.
… macOS

The macOS ARM test build squares the zone size in unsigned 32-bit arithmetic: on the 300 x 300
zone it returns (89999^2 mod 2^32 + 1) / 2 = 1 902 426 353, and on the 250 x 250 zone the square
still fits, so it returns the true 1 953 062 501. The example now accepts the signed wrap quoted on
the page or the unsigned wrap computed from the zone size, and the page names both values.

@vjaganat90 vjaganat90 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through all 186 files and checked the pages against the code: formulas, defaults, settings, column names, IBSI ids, Validation claims against oracle_coverage.csv, and the guide's CLI, CMake, format, GPU, out-of-core and output claims. I re-derived a sample of worked-example values with numpy/scipy, and ran the 2D and 3D worked examples on builds of main, #485 and #486. Most of it holds up. This is what I checked and found right:

  • Generators: both generators' --check really catch drift in names, defaults and staleness, and their tests mutate a default, a cited file and a missing default_at to prove it.
  • plugin.json: the removed feature entries were all invalid on main, and the remaining ones are valid.
  • Build config: the release-please x-release-please-version marker works in CITATION.cff, and autodoc_mock_imports matches .readthedocs.yml.
  • Python API docstrings: the signatures, defaults, group names, Raises sections, ROI blacklist syntax, Nested, ImageQuality, anisotropy and ram_limit all match the code.
  • 3D pages: the formulas, index example values, IBSI ids and Validation claims re-derive and match the registry, apart from the comments below.
  • Guide pages: they match main, apart from the comments below.

Blocking

  1. Executed examples assert undefined behaviour. test_2d_docs_examples_mechanics.py runs in pytest tests/python, so every value a script asserts becomes a CI contract. Three blocks assert the result of UB:

    • fractal_worked_example.py:99-105 keys an out-of-range double → uint32 conversion on platform.machine().
    • glszm_worked_example.py:102-116 pins a signed int square that wraps. It has already needed two follow-up commits to accept other wraps.
    • f_glszm_3d.rst:456-465 quotes a 3D LAE wrap that macOS main doesn't produce.

    The compiler decides these, not the architecture. Please drop those pins and describe the overflow as a defect.

  2. Merge order with #485 and #486. These pages describe main correctly, but both open PRs change what they describe, and the docs checks make that a CI failure, not just stale text:

    • Examples fail. With #485, glszm_worked_example.py and glrlm_worked_example.py fail. With #486, directory_and_params.py (set_params(coarse_gray_depth=256) now raises) and the glrlm_3d, glszm_3d, gldm_3d and ngtdm_3d worked examples fail.
    • --check fails. gen_feature_reference.py --check fails on either PR's registry rows. gen_cli_reference.py --check fails on #486's three new includes in environment.cpp (point 3).
    • Text goes stale with no failure. On the NGLDM/GLDZM/GLCM pages, conventions.rst, cli_reference.rst and the nyxus.py docstrings, nothing fails and the text just goes stale.

    I'd land #485 and #486 first and rebase this on them. The rebase would regenerate the indexes, replace the overflow and binning sections with the fixed behaviour, and pin the new values. The per-page list is at the end of this review.

  3. cli_options.toml cites defaults by line number (inline). Any edit above a cited line in about 25 C++ files fails the Python suite.

  4. The description doesn't match the branch in four places:

    • "The code is unchanged": three ValueError messages in nyxus.py changed.
    • "Each worked-example script asserts every value its page quotes": glszm_3d, glrlm_3d, ngldm and fractal don't.
    • "Each page's Validation section claims only the oracles in the coverage registry": the 2D GLDM, GLDZM, GLSZM, NGLDM and NGTDM pages cite IBSI consensus checks that have no ibsi row. AREA_UM2 is listed as MATLAB-validated but is 0 in every real run. f_gldzm_3d.rst cites regression tests that don't exist.
    • The CLAUDE.md/CONTRIBUTING.md change to the defect-annotation rule is a policy change, not a correction (inline).
  5. EROSIONS_2_VANISH stays vetted after this PR's own rewritten note says the oracle can't tell the two conventions apart (inline).

  6. test_2d_docs_examples_mechanics.py runs 3D and dimension-free scripts under a 2d name (SPEC 6.1; inline).

  7. Statements that are wrong on main (inline):

    • the GLCM values-vs-positions list under PyRadiomics binning;
    • the GLRLM overflow table, which leaves out LRE;
    • coarse_gray_depth / ibsi, which don't say that ibsi=True turns texture binning off;
    • the set_params note (HISTOGRAM does change);
    • the holed square's inner loop, which is 13.66, not 16;
    • the whole-slide fractal value of a filled rectangle;
    • the POWER_SPECTRUM_SLOPE formula, which leaves out an out-of-bounds read;
    • Gabor kernel size 1, which the page calls valid but which reads past the buffer.

Code defects written up as behaviour

Checking the pages against the code turned up about 20 real defects. The pages describe them carefully, but as conventions, and the scripts pin many of them, so each fix will fail the docs suite as if it were a regression. The inline comments give each cause and its fix:

  • Intensity and histograms: HISTOGRAM alone gives zero columns; ENERGY/ROOT_MEAN_SQUARED wrap at 32 bits, in 2D and 3D; the IH_MAX_GRADIENT DBL_MIN seed; IH bin-edge rounding; --coarseGrayDepth=0 writes before the histogram buffer.
  • Box and mode: whole-image and whole-volume boxes are one pixel too large (aabb.h init_from_wh/init_from_whd); 3D shape under single_roi=True uses the bounding box.
  • Morphology: ellipse, circle and hexagonality codes are wrong when requested alone; AREA_UM2 is always 0; the circle/ROI-radius contour offset; Feret samples both 0° and 180°.
  • Erosion: the asymmetric frozen border; the constant-ROI skip of a mask-only feature.
  • Zernike and Gabor: Zernike drops the centre pixel; Gabor's CPU and GPU offsets differ, frequency and angle are swapped, and kernel size 1 reads past the buffer.
  • Image quality and out-of-core: image quality reads uninitialised members out of core; the out-of-core contour offset in moments and radial distribution.
  • Texture: GLCM's per-position overwrite of DIFAVE/SUMAVERAGE/IV; GLCM's constant ROI ignores --noval; 3D NGTDM's IBSI I is never cleared out of core; get_metaparam("3glcm/...") reads the 2D settings.

Please file an issue for each one. Then on the page, call it a defect and link the issue, and add a comment in the script naming the issue next to each assertion that pins one. That way a fix updates the page on purpose.

Nits are inline. That includes the "a fix is coming" promises, about 40 of them, gathered into one comment on installation.rst.

Comments on files GitHub shows without a diff

This PR is past GitHub's diff limit, so these files can't take line comments. Here they are with links to the head commit.

  • nyxus.py:64: coarse_gray_depth doc omits that ibsi=True disables texture binning

    Blocking: This says coarse_gray_depth is the number of grey levels the texture features bin to. With ibsi=True that is not so: the texture families skip binning and read the raw levels (docs/source/cli_reference.rst:150 in this same PR says so for --ibsi, and ibsi_mode.rst:36 too). I checked on this branch: 40x40 random uint16 ROI, Nyxus(["GLSZM_SAE","NGTDM_COARSENESS","GLDM_SDE","GLRLM_SRE_AVE","NGLDM_LDE"], coarse_gray_depth=d, ibsi=True) returns identical values for d=8 and d=64 (0.99718, 0.0015, 0.99438, 0.9993, 0.99438), while with ibsi=False they differ (0.70082 vs 0.95843, ...). Only ENTROPY/UNIFORMITY/HISTOGRAM keep following the depth. Please add this to coarse_gray_depth and to ibsi (which currently reads only 'compute the IBSI-compliant variant'), here and in the Nyxus3D/ImageQuality copies.

  • nyxus.py:196: Note says set_params leaves computed features unchanged; HISTOGRAM changes

    Blocking: This note (repeated at nyxus.py:1111, :1193, :1503, :1960, :2038, :2271, :2974) says changing coarse_gray_depth afterwards 'updates what get_params reports but not the features computed by this instance'. On this branch n = Nyxus(["GLSZM_SAE","ENTROPY","HISTOGRAM"], coarse_gray_depth=64); n.set_params(coarse_gray_depth=8) leaves GLSZM_SAE and ENTROPY at their depth-64 values but the output now has 8 HISTOGRAM_BIN_* columns instead of 64. So the feature set does change (its column count follows the new depth while the binning does not). Please state that, or the note misleads anyone who reads it as 'no effect'. This is the behaviour #486 replaces by refusing a changed depth, so the note needs rewriting after #486 in any case.

  • nyxus.py:322: Code changed, contradicting the 'docstring-only' claim

    Blocking: The description says the docstring-stripped syntax trees of nyxus.py and functions.py equal the previous ones. They don't: this line, nyxus.py:1582 (Nyxus3D) and nyxus.py:2345 (ImageQuality) change the ValueError text from ... must be non-negative. to ... must be positive. (I diffed the ast.unparse of both versions with docstrings removed; these three strings are the only difference). The new wording is more accurate, since 0 is refused, but it is a user-visible message change in a PR that states the code is untouched. Please either say so in the description or drop the three edits. Note that #486 removes these checks (it accepts 0 and negatives), so tests/python/test_api_arguments_mechanics.py, which asserts match="coarse_gray_depth.*must be positive", cannot survive whichever PR lands second.

  • f_ngldm.rst:556-563: Cross-PR #486: NGLDM negative/zero-depth paragraph goes false

    Nit: This paragraph is right for main, but #486 makes every sentence of it false. Whichever of #486 and this PR lands second has to rewrite it. In #486 NGLDM bins 0 and negative depths through the shared scheme (TextureFeature::ngldm_grey_level in texture_feature.h). A negative depth becomes IBSI's fixed bin number over the ROI's [min, max], 0 keeps the raw levels, and Nyxus(coarse_gray_depth=...) accepts both. I measured this page's worked example with a #486 build:

    depth GLM HGLCE levels
    −64 (page: 2.405e9, 6.847e18) 29.68 1241.2 1, 17, 33, 49, 64
    0 (page: 0, 0) 2.8 9.28 1–5, the IBSI-mode values

    #486 makes three other statements on this page true only for a positive depth: the tip at lines 31–32 ("does not use the grey-level binning of the other texture families"), lines 49–50 ("its own rule, not with the binning that GLCM, GLRLM and GLSZM share"), and the Settings row at 164–166 ("A negative value does not select PyRadiomics-style binning"). ngldm_worked_example.py asserts none of these values, because the constructor refuses them on main. So nothing fails after the merge and the page goes stale silently. Once both PRs are in, the script can pin the −64 and 0 values from Python.

  • f_ngldm_3d.rst:483-484: set_params ignores a positive coarse_gray_depth too

    Nit: Same point as on the 3D GLDZM page: "set_params ignores 0" undersells it. On main a positive set_params(coarse_gray_depth=N) is accepted and shows up in get_params(), but the NGLDM grey levels keep the constructor's depth: per-family settings are compiled only in the constructor (src/nyx/python/new_bindings_py.cpp:127). Please say that only the constructor sets the depth, as cmdline_and_examples.rst:218 does.

  • f_ngtdm.rst:20-24: Name the out-of-core IBSI cause instead of "a fix is coming"

    Nit: The behaviour described here is right; I reproduced it with two ROIs out of core in IBSI mode. ROI 1 matched, and ROI 2's coarseness was 0.0405 instead of 0.0669. "(a fix is coming)" will go stale in user docs, though. The cause is that NGTDMFeature::I is a member that clear_buffers() never clears (ngtdm.cpp:19-31), and the IBSI branch appends 0..max to it (ngtdm.cpp:56-61) on the feature instance that the out-of-core path reuses across ROIs. I'd link the tracking issue in place of the promise. It's IBSI-only: at --coarseGrayDepth=0 the rows come from lower_bound and still match, which I checked. f_glcm.rst:789 (constant ROI returns 0 whatever --noval is) makes the same "a fix is coming" promise. There the cause is that calculate() writes the substitute into r.fvals and save_value() then overwrites it with the empty per-angle buffers (glcm.cpp:29-96, :143-208), and the writer pads those with 0.0 (output_2_buffer.cpp:216-217). Naming that is more useful than the promise.

  • f_ngtdm_3d.rst:31-33: A lowest intensity of 0 also reproduces PyRadiomics busyness

    Nit: "with any other lowest intensity, all features but busyness do" is wrong for a lowest intensity of 0. When the cube's lowest level is 0, every level is raised by one (src/nyx/features/3d_ngtdm.cpp:222-228). A ROI of 0..k that fills its box therefore gets levels 1..k+1, which is PyRadiomics' binWidth=1 numbering, and busyness matches too. The vetting phantom is this case: intensities 0..5, busyness vetted at rel 1e-9 (ngtdm3d.pyradiomics_binwidth1 in tests/vetting/config_recipes.md). On the worked example minus 1, 3ngtdm/greydepth=0 gives busyness 1.47697, the page's 1.477. I'd write "whose lowest intensity is 0 or 1 ...; with a higher lowest intensity, all features but busyness do".

  • f_ngtdm_3d.rst:257-258: Complexity reads as divided twice

    Nit: The 2D definition this links to already divides by N_{v,p} (f_ngtdm.rst:256), so "divided by the number of visited voxels" reads as a second division. The code divides once (3d_ngtdm.cpp:473, sum / Nvp), and the worked example's 5.241 is that single division. I'd write "with N_{v,p} the number of visited voxels", matching the Contrast entry below.

  • f_ngtdm_3d.rst:327-329: Constant-ROI-without-corners values need the intensity

    Nit: Contrast 31.52, complexity 18.57 and strength 1.176 depend on the block's intensity: the two levels are the background's 1 and the intensity plus one. The script uses 7 (docs/examples/ngtdm_3d_worked_example.py:92), and the page doesn't name it, so a reader can't reproduce the numbers. Please write "a constant 3 × 3 × 3 block of intensity 7 without its eight corners".

  • f_pixel_intensity.rst:879-881: coarseGrayDepth=0 is an out-of-bounds write; #486 changes all of this

    Nit: On main, "undefined" means memory-unsafe. With --coarseGrayDepth=0, TrivialHistogram::initialize sizes bins_cust_ to 1 and then runs bins_cust_[n_custBins - 1] += ... with index −1 (src/nyx/features/histogram.h:82), a write before the buffer. The CLI accepts 0 silently (environment.cpp:669, no range check) and prints ENTROPY=-0, UNIFORMITY=0. The CLI should refuse it, as Python does, rather than the docs describing a result.

    These sentences also clash with #486, which is open: histogram_bin_count() there maps 0 to 24 bins and a negative N to |N| bins for HISTOGRAM and IH, and Python then accepts every 32-bit integer. If #486 lands first, this paragraph, line 175-177 ("HISTOGRAM writes no columns (a fix is coming)", "Python accepts only positive values"), f_intensity_histogram.rst:185-189 and 731-732 (0 and negatives give the noval substitute), and the HISTOGRAM text in featurelist.rst all go stale. Whichever PR merges second needs to update them.

  • f_radial_distribution.rst:60-62: Whole-slide radial centre lands on the image's top edge, undocumented

    Nit: With the four-corner contour the coarse-to-fine search degenerates, and the page does not say so. For n = 4, max_sqdist/min_sqdist (src/nyx/features/pixel.cpp:41-68, :114-141) start with step = int(4/ln 4) = 2, so they compare corners 0 and 2 only; when corner 0 wins they stop on the window [0, 2) and never look at (W, 0) or (0, H). Every pixel on the perpendicular bisector of the (0,0)-(W,H) diagonal then has spread 0, and find_center (pixel.cpp:146) takes the first one read, on the top rows near the right edge, not the middle of the image. I get the centre at (19, 1) on a 20 × 20 slide (R = 19.03, 11 pixels in ring 0) and at (50, 0) on a 64 × 48 slide (R = 50, 88 pixels in ring 0); featurize_files(..., single_roi=True) returns exactly the ring fractions those centres give. *WHOLESLIDE* includes these features, so please add this under Possible issues with the measured values (the rings are centred on an edge pixel, and the outer rings cover only part of the slide), and file it as a defect.

  • f_radial_distribution.rst:449-450: "in memory" is ambiguous in the tie example

    Nit: Both values here come from the in-memory path; the difference is array input against file input (radial_worked_example.py:178-179). Elsewhere on the page "in-memory" means "not out-of-core", so I'd write "9 for the array and 2 for the same image read from a file".

  • f_radial_distribution.rst:459: Pages disagree on the 15-ellipse out-of-core count

    Nit: This page says 2 of 15 random ellipses at the origin differ out-of-core, but large_images.rst:222-223 says "one or two of 15". It is one measurement, so both pages should quote the same number.

  • f_zernike.rst:408-414: Dropping the centre pixel is a Zernike bug, not a convention

    Nit: zernike.cpp:273 skips r < DBL_EPSILON only because of a = x/r and the H3/r2 term (zernike.cpp:282-283, 322). But R_n0(0) = (-1)^{n/2}, so the pixel does contribute to every m = 0 moment. Every symmetric ROI with an odd side loses its centre pixel, as the 3 × 3 example shows (Z0 0.2387 instead of 0.3183). The fix is small: at r = 0, add const_t * (-1)^(n/2) to AR[n][0] and skip the recurrence.

    I'd file it and label this paragraph as a known defect. zernike_worked_example.py:94-95 pins the wrong value (z[0]*pi == 0.75) and will need to change with the fix.

  • nyxus.py:492: file_pattern is a full match, not stated

    Nit: 'Regular expression selecting the files, matched against file names' does not say the whole name must match. file_pattern=r"c0" selects nothing here and raises RuntimeError: ... no intensity and/or label files to process, while r".*c0.*" works, and the case must match (.*_C0\.ome\.tif finds nothing). Say 'full match, case-sensitive' (same wording at :1744, :2416).

  • nyxus.py:902: Per-file blacklist matches the bare file name only

    Nit: The per-file form file1.ome.tif:5,6,7 only works with the bare file name. blacklist_roi("seg/p0_y1_r1_c0.ome.tif:1,2,3") is accepted without error but removes nothing (139 rows before and after), while p0_y1_r1_c0.ome.tif:1,2,3 removes 3. Please say that a directory part is not matched, since the summary string echoes whatever was passed and gives no hint.

What #485 and #486 make false, page by page

Guide, docstrings, generated reference. All of these are #486, and true on main.

  • conventions.rst:
    • 152-155: only a positive depth is accepted.
    • 158-166: NGLDM, and HISTOGRAM at a negative depth.
    • 177-178: glcm/greydepth.
    • 182-203: the 3D families "0: no binning; --coarseGrayDepth has no effect"; GLDZM at a negative depth.
    • 233-238: the 3D default binning.
    • 333-346: the 3D background rows.
  • cli_options.toml:277-278 and the generated cli_reference.rst:150: "3D GLRLM, GLSZM, GLDM and NGTDM ignore it", the NGLDM rule, HISTOGRAM writing no columns when negative, "8.5 runs as 8" (#486 refuses 1.5).
  • ibsi_mode.rst:68-69; featurelist.rst:148, 2500, 2613, 2678, 2882.
  • nyxus.py:
    • 64, 1400 and 2214: "must be positive"; 3D GLRLM/GLSZM/GLDM/NGTDM "ignore it ... 0 by default"; ENTROPY/UNIFORMITY/HISTOGRAM using coarse_gray_depth bins (#486: abs(depth), 24 at 0).
    • 405-406: glcm/greydepth.
    • The eight set_params Notes: #486 raises for a different depth and drops it from set_environment_params.
  • test_api_arguments_mechanics.py asserts "must be positive"; #486 removes that check.
  • directory_and_params.py:28,34 and cmdline_and_examples.rst:214-236: set_params(coarse_gray_depth=256).

2D feature pages.

  • #485: f_glrlm.rst 265-267, 284-287, 321-323, 341-342, 459-460, 478-481 and 507-531; glrlm_worked_example.py:79-87; f_glszm.rst 284-285, 359-360 and 532-544; glszm_worked_example.py:102-116.
  • #486: f_ngldm.rst 31-32, 49-50, 164-166 and 556-563; f_gldzm.rst 170-173 and 562-570; f_glcm.rst 187-188; "(command line only)" on six pages; f_pixel_intensity.rst 175-177 and 879-881; f_intensity_histogram.rst 185-189 and 731-732.
  • Generated indexes: with #485, the 2D and 3D GLRLM and GLSZM indexes and featurelist.rst; with #486, the 2D and 3D GLDZM and NGLDM indexes.

3D feature pages. I measured these values on a Release build of #486's head, on the pages' worked examples; #485's values are closed form. On the 3D pages #485 breaks no script, because none of the values it changes is asserted, but the text goes stale.

#486, default binning of 3D GLRLM / GLSZM / GLDM / NGTDM moves from unbinned to 64 MATLAB levels

f_glrlm_3d.rst:

  • 34-36, 97-99, 214-218 and 225-228: "no binning by default", "--coarseGrayDepth does not apply". coarse_gray_depth=2 now gives the 3glrlm/greydepth=2 values.
  • 145-146: "default settings and IBSI mode give the same values". The default differs on GLV and the six grey-weighted emphases: GLV 229.38, HGLRE 1706.6 vs 6.435. The index values are IBSI-mode values; say so in the caption.
  • 102-104 and 177-178: background voxels form level-1 runs, and N_g counts the bounding box's levels. Background is now level 0 from the voxel cloud, and N_g counts the ROI's levels.
  • 399-401 and 460-474 (RP above 1):
    • two-corner mask at greydepth=2: RP 0.72, LGLRE 0.4583 (was 0.8 / 0.5125);
    • ellipsoid: RP 0.9396 / 0.9850 / 0.9926 at 16 / 64 / 128, RP_AVE 0.9854 at 64 (was 1.131 / 1.182 / 1.191 and 1.130);
    • "without binning" now needs greydepth=0.
  • 476-479: "the intensity by default". It is now the bin number by default.
  • 499-500: "Without binning, the default".
  • 515-516: #486 adds TEST_3D_GLRLM_BOUNDED_FEATURES_INVARIANT.
  • glrlm_3d_worked_example.py 53-57, 64-69, 79-83 and 84-86 fail.

f_glszm_3d.rst:

  • 31-33, 49, 178-184 and 195: "0 by default", "--coarseGrayDepth has no effect". coarse_gray_depth=8 gives GLV 4.617, HGLZE 38.0.
  • 61-62, 141-145 and 399-401: "background is 1 with MATLAB-style binning", "levels present in the bounding box". Background is level 0, and level-1 voxels form zones.
  • 92-93: the default matrix has rows 17, 33, 49 and 64, not 1 to 4. The default GLV is 389.36 vs 1.580 in IBSI mode.
  • 420-428 ("drops the lowest level"): fixed. greydepth=2 gives 4 zones, ZP 0.1481, SAE 0.5283, LAE 123.75, LGLZE 0.8125.
  • 441-445 ("ignored in whole-volume mode"): whole-volume now reads 3glszm/greydepth, ZP 0.1481.
  • 482: MATLAB-style binning is now the default.
  • glszm_3d_worked_example.py 83-85, 97-101 and 121-128 fail.

f_gldm_3d.rst:

  • 32-35, 51, 182 and 190: "0 by default", "--coarseGrayDepth has no effect". coarse_gray_depth=8 gives GLV 3.757, HGLE 36.10.
  • 63 and 139: "background is level 1 when N > 0", "levels in the bounding box".
  • 93: the default rows are 17, 33, 49 and 64.
  • 367-374 (dimmest voxels dropped): fixed.
    • greydepth=2: Nz 48, SDE 0.02213, LDE 116.08, GLN 29.04, GLV 0.1975.
    • greydepth=1: computed, SDE 0.006641, LDE 239.4.
  • 376-381: default HGLE 1916.8 on both volumes.
  • 384: an intensity-0 ROI voxel is counted under the default (SDE 0.08743, LDE 32.96).
  • 414: "the default (no binning)".
  • gldm_3d_worked_example.py 66-69, 73, 82-83, 85 (get_metaparam now returns 64), 89 and 123 fail.

f_ngtdm_3d.rst. #486 changes the default but not NGTDM's background handling.

  • 30-33, 46, 63, 82, 105-106, 172, 185-186 and 205: the default is 64 levels, and --coarseGrayDepth applies. coarse_gray_depth=8 gives coarseness 0.08807, busyness 1.518.
  • Default values on the worked example: coarseness 0.009831, contrast 734.1, busyness 1.485, complexity 1294.3, strength 11.13.
    • They change ngtdm_3d_index.inc 17-37 and [ngtdm_3d] in feature_extras.toml 628-632, unless the page moves its examples to 3ngtdm/greydepth=0.
  • 228-231: default busyness 1.4846; the shift by 9 gives 0.9094.
  • 294-305: under the default, voxels outside the mask and intensity-0 voxels are level 1 and are not visited. Corners off gives 0.01215 / 551.8 / 1.062 / 1486.7 / 15.39.
  • 317 (radius 2): 0.01005 / 722.8 / 1.453 / 1276.8 / 11.30.
  • 327-329 (constant ROI, corners off): coarseness 0.00407, the other four 0.
  • 340: the default rows are the bins present.
  • 353: the default output is now unvetted; the vetted rows are at greydepth=0.
  • ngtdm_3d_worked_example.py 36, 61, 64-67, 73-75, 81 and 97 fail (default-settings checks; the IBSI checks pass).

#486, signs of the coarse grey depth, and set_params

  • f_intensity_3d.rst 49, 162 and 185: depth 0 gives 24 histogram bins (main writes before the buffer). Python accepts any 32-bit integer.
  • f_glcm_3d.rst 187-188 and 192: Nyxus3D no longer rejects a depth below 1.
  • f_gldzm_3d.rst:
    • 194: "Python accepts only a positive value". Depth 0 gives GLM 3.286, and -8 gives 5.286.
    • 472-474: set_params with a depth other than the current one now raises ValueError.
    • 485-493: #486 adds a third, re-binning MIRP configuration (gldzm3d.mirp_fbn20_negdepth, 16 vetted rows).
  • f_ngldm_3d.rst:
    • 172: a negative depth now selects PyRadiomics-style bins. At -20: GLM 9.889, HGLCE 147.2.
    • 467: the other four 3D families now bin at the coarse depth.
    • 480-484: 0 gives the raw levels (GLM 2.407, the IBSI value), not every voxel at level 0. A negative depth no longer wraps to 2³²−|N|.
    • 504: a third MIRP setting, ngldm3d.mirp_fbn20_negdepth.

#485

  • f_glrlm_3d.rst:
    • 264-266: "with the differences in integer arithmetic noted below".
    • 305-306: "Unlike the 2D code". 2D HGLRE is squared in double too.
    • 338-340: LRHGLE becomes 1.2308 × 10⁹ instead of 1.0529 × 10⁹.
    • 443-445: SRLGLE becomes 3.374 × 10⁻⁹ instead of 0.
    • 481-500: the overflow section and its table go entirely.
    • The index Validation cells of the eight emphases gain the analytic rows.
  • f_glszm_3d.rst 456-465: LAE becomes 2 047 936 001 and SAE 0.5. The Validation section should name TEST_3D_GLSZM_LARGE_ZONE_SIZE_EMPHASIS_ANALYTIC, and the index cells of SAE and LAE gain analytic (rel. 1e-12).

Unaffected by both:

  • every IBSI-mode example value in feature_extras.toml;
  • the 3D shape page;
  • the 3D GLCM values;
  • GLRLM greydepth=2 without a mask hole;
  • GLSZM greydepth=-3.

Comment thread docs/cli_options.toml Outdated
group = "Texture settings"
required = false
default = "64"
default_at = "environment.cpp:106"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: default_at (here environment.cpp:106, and the same for --dim, --resultFname, --ibsi, --pixelDistance, --reduceThreads, ...) names one source line, and gen_cli_reference.py --check (also run by test_docs_cli_reference_table_matches_code_mechanics on every pytest tests/python run) fails when that line does not assign the default. Inserting an include or a line above line 124 in environment.cpp is enough. #486 adds three #include lines at the top of environment.cpp and rewrites the --coarseGrayDepth parsing; I ran this branch's docs/ against #486's src/ and --check reports --dim, --resultFname, --ibsi, --pixelDistance, --coarseGrayDepth and --reduceThreads as 'not the value assigned'. So every PR touching environment.cpp, environment_basic.h or dirs_and_files.cpp fails the Python suite until someone renumbers this table by hand, which punishes unrelated code changes. Locate the assignment by symbol instead (a member name, e.g. default_at = "environment.cpp:n_pixel_distance" matched by regex, or read the value from the member's initialiser) so a code move does not break the check, while a changed default still does.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed default_at now names the variable or call that is assigned the default (environment.cpp:n_pixel_distance), and --check requires exactly one literal assignment of it in that file. New tests add lines above every default (still passes), change a default in a copy of the source (caught) and add a second assignment (refused). Done in 8dfe1b0.



@pytest.mark.parametrize("script", _params())
def test_2d_docs_examples_run_mechanics(script, tmp_path, extras_json):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: test_2d_docs_examples_run_mechanics runs every script in docs/examples/, including shape_3d_worked_example.py, glcm_3d_worked_example.py, volume_files.py, volume_channels.py and so on, and test_2d_readme_quick_starts_run_mechanics runs the README blocks whatever they use. SPEC 6.1 makes the dim token name the implementation under test and requires a function's token to match its file's. Either split into test_2d_docs_examples_mechanics.py and test_3d_docs_examples_mechanics.py (selecting scripts by name), or follow what you did for test_docstring_examples_mechanics.py and list this file in DIM_AGNOSTIC in tests/vetting/check_test_names.py with a reason and drop the 2d token. As it stands check_test_names.py passes only because the checker cannot see which scripts the parametrisation selects.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed Took the second option: the file is now test_docs_examples_mechanics.py, its functions dropped the 2d token, and it is listed in DIM_AGNOSTIC with the reason that it runs 2D, 3D and dimension-free scripts alike. Every reference to the old name in the scripts and the developer guide is updated. Done in 8dfe1b0.

run: python -m sphinx -b html -W --keep-going docs/source docs/build/html

- name: Check links
run: python -m sphinx -b linkcheck -W --keep-going docs/source docs/build/linkcheck

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: linkcheck -W fails the job on any transient 403/429/timeout. The pages link to ncbi.nlm.nih.gov, imagej.net, github.com/PolusAI (many anchors) and others, all of which rate-limit or time out from GitHub runners now and then, so an unrelated PR gets a red check. Make the link check a separate non-required job (or continue-on-error, or a weekly schedule: trigger), and set linkcheck_retries/linkcheck_timeout and linkcheck_ignore for the rate-limited hosts in conf.py, which sets none of them. Also add permissions: contents: read (the other release workflows set permissions; this one gets the default token scope) and pip caching (cache: pip on setup-python).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed The link check is now its own Check_Links job with continue-on-error: true, so a rate-limited host no longer fails the workflow, and conf.py sets linkcheck_retries, linkcheck_timeout and linkcheck_rate_limit_timeout instead of an ignore list. The workflow also gets permissions: contents: read and pip caching. Done in 8dfe1b0.

- 'src/nyx/featureset.cpp'
- 'src/nyx/env_features.cpp'
- 'src/nyx/features/*.h'
- 'tests/vetting/oracle_coverage.csv'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: gen_cli_reference.py --check reads environment.cpp, environment_basic.h, dirs_and_files.cpp, main_nyxus.cpp, features/*.cpp etc. (every file named in an evidence/default_at), but the filter lists only cli_option_constants.h, featureset.*, env_features.cpp and features/*.h. A PR that changes a default in environment.cpp does not run this workflow; the drift is then caught only by the pytest wrapper, and the docs build and the staleness check of cli_reference.rst never run for it. Add src/nyx/*.cpp/*.h (or src/nyx/**) to both filters.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed Both path filters now use src/nyx/** in place of the hand-picked files, so a PR that changes any cited source file runs the docs workflow. Done in 8dfe1b0.

Comment thread environment.yml
@@ -17,17 +16,11 @@ dependencies:
- numpy
- pip>=20.1
- pyarrow
- fmjpeg2koj
- fmjpeg2koj
- arrow-cpp[version='>=10.0.0']

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This drops the https://repo.continuum.io/pkgs/free channel and parquet-cpp from the build environment in addition to the sphinx entries. Docs-tool removal is fine (they move to docs/requirements.txt), but parquet-cpp is a build dependency of the Arrow/Parquet path and its removal is a build-environment change that the description does not mention. Keep that out of this PR or say why it is safe (conda-forge libparquet/arrow-cpp provide it).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed Restored the repo.continuum.io/pkgs/free channel and parquet-cpp; only the sphinx entries leave environment.yml, so the build environment is unchanged. Done in 8dfe1b0.

Comment on lines +348 to +353
Different default pairs on the command line and in Python
Without ``--gaborfreqs`` and ``--gabortheta``, the command line pairs frequencies
:math:`0, \pi/4, \pi/2, 3\pi/4` with orientations of 4, 16, 32 and 64 radians: the built-in list
stores each frequency where an orientation belongs, and the reverse. Python uses frequencies 4,
16, 32, 64 at 0°, 45°, 90°, 135°. The same image gives different ``GABOR`` columns from the two
unless both lists are given.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Three items under Possible issues are bugs, not conventions:

  • The built-in pairs have frequency and angle swapped (gabor.cpp:19-25).
  • The CPU path reads at ceil(n/2) (gabor.cpp:493,496) while the GPU reads at floor(n/2) (gpu/gabor.cu kerCalcEnergy, halfpad = kerside / 2). So for odd sizes the two paths count different pixels.
  • The GPU path has no constant-baseline guard (gabor.cpp:274-306, compared with 89-95).

None of them has an upstream issue. I'd file each one and say on the page that it is a known defect, with the link. Otherwise the fix will look like a behaviour change. The docs test pins one of them: gabor_worked_example.py:67-68 asserts the shifted kernel-size-15 values, so it will fail when the offset is fixed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed Filed #504 (pairs swapped), #505 (CPU/GPU offsets) and #507 (GPU constant-baseline guard). Each Possible issues item calls its defect known and links the issue, and the kernel-size-15 assertion carries a comment naming #505. Done in ff8f931.

Comment thread docs/examples/zernike_worked_example.py Outdated

# Z0 is 1/pi when every pixel lies in the disk; Z1 is 0 because the disk sits on the centroid
assert math.isclose(features["ZERNIKE2D_Z0"][0], 1 / math.pi, rel_tol=1e-12)
assert abs(features["ZERNIKE2D_Z1"][0]) < 1e-16

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: ZERNIKE2D_Z1 is pure rounding residue. Its size depends on summation order and FMA contraction, which differ between compilers and architectures. Here (arm64) it is 2.0e-17 on the example and 4.0e-17 on its rot90, so the < 1e-16 bound leaves a margin of 2.5-5×. The analytic test uses a 1e-15 floor for the same column (tests/test_2d_zernike_analytic.h:58). I'd use 1e-15 here, and on the page (f_zernike.rst:89-90) say "a rounding residue of order 10^-17" rather than quote a bound that only one platform shows.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed The bound is now 1e-15, and the page says "a rounding residue of order 10^-17". Done in 26d3205.

Comment thread docs/source/Math/f_gabor.rst Outdated
Comment on lines +403 to +405
at output time (by ``--noval`` in CSV and pandas, by ``-0.0`` in Arrow and Parquet). ROIs
whose padded image times the number of filters exceeds cuFFT's limit of :math:`2^{27}` values
are refused. The GPU values are not checked in continuous integration.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: conv_dud_gpu_fft_multi_filter refuses when 2 * size * n_filters >= 2^27 (src/nyx/gpu/gabor.cu:343-344). The 2 counts real and imaginary doubles, and n_filters is pairs + 1 because the baseline is one of the filters (gabor.cpp:255-269). A ROI is therefore refused at (H+n-1)(W+n-1) * (pairs+1) >= 2^26 complex values, not at 2^27. Also, on the command line "refused" means GaborEnergyGPUMultiFilter returns early with all-zero responses, so every column becomes 0/0 and is written as the --noval value. Only the Python build throws (gabor.cu:347-349).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed The entry now gives the refusal at (H+n−1)(W+n−1) × (pairs + 1) reaching 2^26 complex values, half of cuFFT's 2^27 doubles, and says a refused ROI gets all-zero responses, written as that 0/0 result. Done in 26d3205.

Comment thread docs/examples/gabor_worked_example.py Outdated
Comment on lines +59 to +61
# The baseline wavelength is 62.8 pixels; settings are read in single precision
assert round(2 * math.pi / 0.1, 1) == 62.8
assert f"{float(np.float32(0.1)):.9f}" == "0.100000001"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: f"{float(np.float32(0.1)):.9f}" == "0.100000001" passes whatever Nyxus does with the setting, so it doesn't back the page's claim that every Gabor value is read in single precision (f_gabor.rst:282-283). Nyxus reports the value it stored. nyx.get_params("gabor_gamma")["gabor_gamma"] == float(np.float32(0.1)) holds today (it returns 0.10000000149011612), and so does the same check on gabor_freqs. Asserting that would test the claim.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed The script now reads gabor_gamma and gabor_freqs back with get_params and asserts that they equal their single-precision values. Done in 26d3205.

Comment on lines +70 to +72
# A ROI of one intensity reports 0, whatever its shape
for m in (mask, disk(6), np.ones((7, 9))):
assert erosions(m, values=np.full(np.pad(m, 3).shape, 5, dtype=np.uint32)) == 0, "constant ROI"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: EROSIONS_2_VANISH reads only the mask (erosion.cpp:36 sets every ROI pixel to 1), yet ErosionPixelsFeature::parallel_process_1_batch skips a ROI with aux_min == aux_max (erosion.cpp:158), and osized_calculate copies the skip (erosion.cpp:120). So featurize(mask, mask), or any binary or label image passed as the intensity, reports 0 for every ROI, while the same shape with textured pixels reports 6 (the disk of radius 6). The skip is a leftover of the intensity families' degenerate-ROI guard, not part of the definition, and whole-slide mode already does not apply it (page lines 292-297), so the same shape gives different values in the two modes. The page describes it accurately (lines 61-62, 204-205, 285-290), but these three assertions make the fix (drop both checks) fail the docs suite. I'd file it, link it from the Possible issues entry, and say in the script that these lines pin a known defect. conventions.rst's constant-ROI table (Erosion row) needs the same link.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed Filed #502. The single-intensity bullet, the formula note, the Possible issues entry and both conventions.rst rows (zero intensity and constant ROI) call it a known defect with the link, and the constant-ROI assertion carries a comment naming it. Done in ff8f931.

Demian Vladi added 3 commits October 16, 2026 11:10
…g CI

gen_cli_reference.py --check located each documented default by a line
number, so adding a line above it in any of about 25 source files failed
the Python suite. Each default_at now names the variable or call that is
assigned the default ("environment.cpp:n_pixel_distance"), and the check
requires exactly one literal assignment of it in that file. Moving code
no longer fails the check; changing a default or assigning it twice does.
Tests copy src/nyx, insert lines, change a default and add a second
assignment.

The docs workflow runs for every change under src/nyx, which the
generators read, takes read-only permissions and caches pip. The link
check is its own job with continue-on-error, and conf.py retries links
with a longer timeout. docs/requirements.txt bounds every package.

The docs example test is test_docs_examples_mechanics.py, listed in
DIM_AGNOSTIC: it runs 2D, 3D and dimension-free scripts alike.

environment.yml keeps parquet-cpp and its channel list; only the Sphinx
packages leave it, for docs/requirements.txt.

The --gaborkersize entry says 1 reads past the convolution buffer.
Registry:
- EROSIONS_2_VANISH is a regression pin. scikit-image's count agrees on
  the test shape with another structuring element and counting rule, so
  it confirms neither; the assertion moves to the regression file.
- The six 2D GLRLM grey-level emphases name PyRadiomics, the source of
  their 1e-9 tolerance; IBSI's consensus values agree only to 1e-2.

Worked-example scripts:
- fractal: drop the 4294967295 in-memory pin, an out-of-range
  double-to-uint32 conversion whose result the compiler decides; assert
  0 for a 2 x 2 square without its sign; add the anisotropic disk in
  memory and out of core, and a 256 x 256 whole-slide image (1.8667).
- ngldm, glszm_3d, glrlm_3d: assert the quoted values they skipped; the
  headers name what stays unasserted (32-bit overflow results,
  command-line-only depths).
- gabor: assert the stored single-precision settings; zernike: bound
  the Z1 rounding residue at 1e-15.

Pages:
- GLCM: positional features under PyRadiomics binning (DIFENTRO,
  SUMENTROPY; IDN/IDMN divide by the levels present), DIFAVE,
  SUMAVERAGE and IV as a known defect, and the diagonal labels.
- Morphology: the holed square's inner loop is 13.66; AREA_UM2 is 0
  because the resolution gate is never set, and its MATLAB check sets
  it itself.
- Fractal: the whole-slide box is one pixel larger than the image.
- Image quality: POWER_SPECTRUM_SLOPE reads past its radius table.
- Gabor: kernel size 1 reads past the buffer; the GPU size limit.
- 2D GLDM, GLDZM, GLSZM, NGLDM, NGTDM, GLRLM: comparisons with IBSI
  consensus values are consistency checks, not registry validations.
- 3D GLDZM: merging PyRadiomics binning is not tested.
- Wording and numbers on the GLSZM, GLDZM, 3D intensity, 3D GLCM,
  3D NGTDM, radial, moments, histogram, Zernike and erosion pages, the
  feature-page template sentence, and the README and landing page.
- Supported formats and the OME-Zarr fixture README: bioformats2raw 0.9
  writes big-endian arrays, 0.12 little-endian (both measured).

Docstrings: ibsi=True skips texture binning; file_pattern must match
the whole name; a per-file blacklist names the mask file alone.
Every defect the pages describe now says so and links its issue
(PolusAI#489-PolusAI#528), and every "a fix is coming" with an issue behind it is
replaced by the link; packaging gaps are called known issues. The
worked-example scripts name the issue next to each assertion that pins
a defect's current value.

New Possible issues entries, with measured values: 3D shape under
single_roi=True (the merged mask's bounding box), morphology codes that
need companion families, the whole-slide extrema, the whole-slide
radial centre, and the 3D GLCM metaparameter getter.

The "is coming" lines left describe behaviour that PolusAI#486 changes.
@darkclad

Copy link
Copy Markdown
Author

Blocking 1 (UB asserts) — Deferred The fractal pin is gone and the conversion is filed as #515 in 26d3205 and ff8f931. #485 fixes the two GLSZM squares, so after #485 and #486 merge, this PR rebases onto them and replaces glszm_worked_example.py:102-116 and f_glszm_3d.rst:456-465 with the true values, pinned.

Blocking 2 (merge order) — Deferred Agreed on the order: after #485 and #486 merge, this PR rebases onto them, regenerates the indexes, rewrites the overflow and binning sections to the fixed behaviour and pins the new values, page by page from your list.

Blocking 3 (default_at) — Fixed default_at now names the assigned variable or call instead of a line, with tests for moved code, a changed default and an ambiguous target. Done in 8dfe1b0.

Blocking 4 (description) — Fixed The description now states the ValueError text change, says which values each script header leaves unasserted (the scripts gained the missing assertions in 26d3205), calls the IBSI comparisons consistency checks, says the AREA_UM2 check covers the formula but not the program, and corrects the regression claim on f_gldzm_3d.rst. The policy change stays and has its own heading; see the CLAUDE.md thread.

Blocking 5 (EROSIONS_2_VANISH) — Fixed The row is regression and its assertion moved to test_2d_morphology_regression.h, with the index and page updated. Done in 26d3205.

Blocking 6 (2d test name) — Fixed The file is now test_docs_examples_mechanics.py, listed in DIM_AGNOSTIC with its reason. Done in 8dfe1b0.

Blocking 7 (wrong on main) — Fixed The GLCM position list, coarse_gray_depth/ibsi, the holed square, the whole-slide fractal value, POWER_SPECTRUM_SLOPE and Gabor size 1 are corrected in 26d3205. The GLRLM LRE row and the set_params note describe what #485 and #486 change, so they are rewritten at the rebase onto them.

Code defects — Fixed Every defect has an upstream issue (#489-#516, and #517-#528 for the release and output promises), each page calls it a known defect with the link, and each assertion that pins one carries a comment naming the issue. Done in ff8f931. The --coarseGrayDepth=0 write is fixed by #486, so it has no issue of its own.

nyxus.py:64 — Fixed coarse_gray_depth and ibsi in Nyxus and Nyxus3D now say that ibsi=True turns texture binning off while the histograms follow the depth. ImageQuality documents both as accepted and unused, so it needs no note. Done in 26d3205.

nyxus.py:196 — Deferred #486 makes set_params refuse a changed depth, so all eight Notes are rewritten anyway. After #485 and #486 merge, this PR rebases onto them and rewrites the Notes to that behaviour.

nyxus.py:322 — Fixed Kept the three strings and stated the change in the description, including that it is the only difference in the docstring-stripped trees. The test_api_arguments_mechanics.py match is settled at the rebase onto #486.

f_ngldm.rst:556-563 — Deferred After #485 and #486 merge, this PR rebases onto them, rewrites this paragraph and lines 31-32, 49-50 and 164-166, and pins the −64 and 0 values from Python.

f_ngldm_3d.rst:483-484 — Deferred #486 makes set_params raise for a different depth. After #485 and #486 merge, this PR rebases onto them and states that only the constructor sets the depth.

f_ngtdm.rst:20-24 — Fixed The promise is replaced by the cause (the grey-level list is not cleared between out-of-core ROIs in IBSI mode) and #512. f_glcm.rst:789 names its cause and links #511. Done in ff8f931.

f_ngtdm_3d.rst:31-33 — Fixed Now reads "whose lowest intensity is 0 or 1 ...; with a higher lowest intensity, all features but busyness do". Done in 26d3205.

f_ngtdm_3d.rst:257-258 — Fixed Now "with N_{v,p} the number of visited voxels", as in the Contrast entry. Done in 26d3205.

f_ngtdm_3d.rst:327-329 — Fixed Now "a constant 3 × 3 × 3 block of intensity 7 without its eight corners". Done in 26d3205.

f_pixel_intensity.rst:879-881 — Deferred #486 maps 0 to 24 bins and accepts negatives, which removes the out-of-bounds write. After #485 and #486 merge, this PR rebases onto them and rewrites this paragraph, lines 175-177, the histogram page and the HISTOGRAM row.

f_radial_distribution.rst:60-62 — Fixed Filed #516 and added "Whole-slide centre on the top edge" under Possible issues with the measured centres (19, 1) and (50, 0), linked from the bullet and large_images.rst. Done in ff8f931.

f_radial_distribution.rst:449-450 — Fixed Now "9 for the array and 2 for the same image read from a file". Done in 26d3205.

f_radial_distribution.rst:459 — Fixed large_images.rst now says "2 of 15", as this page does. Done in 26d3205.

f_zernike.rst:408-414 — Fixed Filed #503, the entry calls it a known defect with the link, and the z[0]*pi == 0.75 assertion carries a comment naming the issue. Done in ff8f931.

nyxus.py:492 — Fixed file_pattern now says it must match the whole file name, case-sensitively, in all three places. Done in 26d3205.

nyxus.py:902 — Fixed Both blacklist_roi docstrings now say a per-file list names the mask file without its directory, and that a name with a directory part is accepted but matches nothing. Done in 26d3205.

@vjaganat90 vjaganat90 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-checked 8dfe1b0, 26d3205 and ff8f931 against every thread and against the points in my review body. I also ran the docs suite and the 2D/3D worked examples on a build of this head. Most of it is done:

  • default_at now names the assignment. gen_cli_reference.py --check passes with #486's environment.cpp and still fails on a changed default.
  • The description now matches the branch.
  • EROSIONS_2_VANISH is a regression row.
  • The examples test is DIM_AGNOSTIC.
  • #489–#528 exist and match the defects that cite them.
  • The fractal UB pin is gone.

I'm fine with deferring the #485/#486 items to the rebase, as long as this lands after both. That includes three things that are wrong today:

  • glszm_worked_example.py:102-116 still asserts a signed-overflow result. On macOS arm64 the 300 × 300 case gives 1902426353, not the page's -245057295.
  • f_glszm_3d.rst:458-465 quotes an LAE wrap that this build doesn't produce. It returns the true 2047936001.
  • The f_glrlm.rst overflow section still leaves out LRE.

So this isn't an LGTM yet. I'll re-review after the rebase.

Blocking

  • f_glcm.rst:801-809: JVAR and DIFVAR are still listed as position-based. Both subtract a mean computed from values (thread reply).

Nits

  • Thread replies:
    • the six GLRLM _AVE registry rows;
    • the 90° neighbour;
    • the NGLDM ladder asserts;
    • #498/#500 in the morphology loop;
    • the 3D ball under #494;
    • the 3D GLSZM header.
  • Inline:
    • three fractal assertions pin defects without naming them;
    • the morphology and radial headers claim values they don't assert.
  • cli_options.toml evidence: the ranges are still line numbers, and --check only checks that a range exists, so they go stale silently. With #486, every range past environment.cpp:665 is 6 lines early. Please re-cite them at the rebase.

The CLAUDE.md/CONTRIBUTING.md change now has its own heading as a policy change. I've left that thread open for the maintainers.

for row in from_files(labels, (labels > 0) * 100):
check(row[:2], [1.8416, 1.0], "two labels")
check(from_files(labels, (labels > 0) * 100, mergerois=True)[0][:2], [1.4231, 1.107], "merged")
check(from_files(np.ones((200, 200)), single_roi=True)[0][:2], [1.8703, 1.0], "whole slide")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The description says each assertion that pins a defect names its issue, and the 256 case below does, but three here don't. Line 161 is the 200 × 200 whole-slide value 1.8703 (#493; it equals the mask-of-ones value only because 201 and 200 both round up to a 256 canvas). Lines 145 and 155 are the out-of-core two squares 1.0 and disk 1.0482 (#509). Please add the # pins known defect comment above each.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed The two out-of-core assertions now carry the #509 comment, and the 200 × 200 whole-slide one carries #493 with the note that 201 and 200 round up to the same 256 canvas. Done in c6f8475.

@@ -0,0 +1,159 @@
# Example for docs/source/Math/f_morphology.rst: 2D morphology features of the page's worked
# example, a notched ROI (label 1) with a small rectangle touching it (label 2). The page shows the
# lines between the "example" markers. The assertions below pin every value the page quotes, so the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This header says every value the page quotes is pinned, but two new entries aren't. The "Codes that need companions" entry in f_morphology.rst quotes MAJOR_AXIS_LENGTH 39.36, ORIENTATION 45.93, ROUNDNESS 0, DIAMETER_CIRCUMSCRIBING_CIRCLE 35.61 and HEXAGONALITY 0 when requested alone, and the whole-slide entry quotes the 64 × 48 extrema. I reproduced all of them, so they're right today, but no assertion would catch a change. The same goes for radial_worked_example.py: the new #516 entry's centres (19, 1) and (50, 0) and the ring-0 counts 11 and 88 aren't asserted. Please assert them, with the issue comments.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed The morphology script now asserts the companion-less ellipse, circle and hexagonality values (#495) and the 64 × 48 whole-slide PERIMETER and extrema (#493). The radial script checks the whole-slide centres (19, 1) and (50, 0) by matching Nyxus's ring counts, ring 0 being 11 and 88 (#516). Done in c6f8475.

Pages:
- GLCM (2D and 3D): JVAR and DIFVAR join DIFAVE, SUMAVERAGE and IV
  under PolusAI#510. JVAR centres positions on JAVE, a mean of values, and
  DIFVAR centres position differences on DIFAVE, so neither is
  position-based when the levels present are not consecutive.
- GLCM directions: name the neighbour of every angle. 90 degrees pairs
  a pixel with the one below, so it is transposed against MATLAB,
  PyRadiomics and IBSI as the diagonals are; only 0 degrees matches.
- NGLDM: the two grey-level ladders are stated as what the formulas
  give, not as Nyxus output.

Registry: the six 2D GLRLM grey-level emphasis _AVE rows name
PyRadiomics, as their base rows do; featurelist.rst and the report
are regenerated.

Worked-example scripts:
- shape_3d: assert the radius-5 ball as a segmented ROI and under
  single_roi=True (PolusAI#494); the 3 x 2 x 2 block fills its box, so it no
  longer carries the issue comment.
- morphology: assert the codes requested without their companion
  family (PolusAI#495) and the 64 x 48 whole-slide perimeter and extrema
  (PolusAI#493); feature_extras.toml names the issue above each ROI 1 value
  that pins PolusAI#497, PolusAI#498 or PolusAI#500.
- radial: assert the whole-slide centres (19, 1) and (50, 0) through
  the ring counts Nyxus reports (PolusAI#516).
- fractal: name PolusAI#509 and PolusAI#493 above the three assertions that pin them.
- glszm_3d: the header names the level-1 values as unasserted.
- ngldm: drop the two ladder assertions, which never called Nyxus.
@darkclad

Copy link
Copy Markdown
Author

Blocking (JVAR and DIFVAR) — Fixed Both are now in the #510 sentence on the 2D and 3D pages, each with the mean it subtracts, and #510 lists all five features. Done in c6f8475.

Nits (thread replies and inline) — Fixed All nine threads are answered in place: the _AVE registry rows, the 90° neighbour, the NGLDM ladders, the morphology loop, the 3D ball, the 3D GLSZM header, the fractal comments and the morphology and radial assertions. Done in c6f8475.

cli_options.toml evidence — Deferred After #485 and #486 merge, this PR rebases onto them and re-cites every evidence range against the rebased environment.cpp.

Wrong today (GLSZM overflow, 3D LAE wrap, GLRLM LRE) — Deferred All three get rewritten at that rebase, along with the round 1 deferred items, and their new values get pinned there.

@vjaganat90 vjaganat90 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-checked c6f8475 against my last round, and everything I raised there is done. All nine threads are settled:

  • JVAR and DIFVAR, the blocking item, are now in the #510 sentence on both GLCM pages, each with the mean it subtracts. The 3D code mixes them the same way as the 2D code (3d_glcm.cpp:642-649, 1066-1077).
  • The 90° neighbour is fixed.
  • The six GLRLM _AVE registry rows are fixed, and the report is regenerated.
  • The NGLDM ladders are fixed.
  • The morphology loop's #497, #498 and #500 marks are in.
  • The 3D ball is asserted under #494.
  • The 3D GLSZM header is fixed.
  • The fractal issue comments are in.
  • The morphology and radial assertions are in.

I built this head and ran the docs examples, the full pytest suite and the four --check generators. All pass, apart from the Arrow tests (my build has no Arrow), and no tracked file was rewritten. One nit inline.

This still isn't an LGTM, only because of the rebase. #485 and #486 haven't merged, so four items wait on it:

  • the three values that are wrong today: the GLSZM overflow example, the 3D LAE wrap, and LRE in the GLRLM overflow section;
  • the cli_options.toml evidence ranges.

I'll approve once this is rebased onto both.

r = Nyxus3D(["*3D_ALL_MORPHOLOGY*"]).featurize_files(["./ball5.ome.tif"], ["./ball5_mask.ome.tif"],
single)
for code, value in zip(codes, expected):
assert math.isclose(r[code][0], value, rel_tol=5e-4, abs_tol=1e-12), f"single_roi={single} {code} = {r[code][0]}, page says {value}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The assert message sits on the same line after a run of spaces (152 columns). It looks like a lost line break. Please move the f-string onto its own line, as the assert at line 45 does:

        assert math.isclose(r[code][0], value, rel_tol=5e-4, abs_tol=1e-12), \
            f"single_roi={single} {code} = {r[code][0]}, page says {value}"

The message of the single_roi ball assertion in
shape_3d_worked_example.py sat after a run of spaces on the assert's
line. It now follows a line continuation, as the other assertions in
the script do. The parsed code is unchanged.
@sameeul
sameeul merged commit 73e23b0 into PolusAI:main Oct 22, 2026
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants