Skip to content

madspace: audit comments and docstrings against the source - #364

Draft
theoheimel wants to merge 2 commits into
mainfrom
audit-madspace-docs
Draft

theoheimel wants to merge 2 commits into
mainfrom
audit-madspace-docs

Conversation

@theoheimel

Copy link
Copy Markdown
Contributor

Summary

Systematic audit of the comments and docstrings in madspace/ (C++ headers and sources, kernels, Python bindings and loader, build scripts, instruction_set.yaml descriptions, tests). The goals were to remove factual errors, since the header Doxygen comments and the YAML descs become the user-facing Python docstrings, and to cut wall-of-text comments inside function bodies.

Comments only. No code was changed. Two accidental code edits made during the audit were reverted, and the diffs were checked mechanically: C++ diffs contain only comment lines, Python diffs are AST-identical apart from docstrings, and instruction_set.yaml is structurally identical with the desc keys stripped.

Two commits, one per pass:

  1. Audit pass: every comment checked against the source, wrong ones fixed, in-function comments shortened, history wording ("now", "previously", "no longer", "as before", ...) and commented-out code removed.
  2. Verification pass: a second, independent re-read of all comments with edits restricted to factual errors only (this pass also caught a few first-pass fixes that had been lost or introduced errors).

Stats

Counts are tallied from the audit agents' reports, not from an exhaustive static count, so treat them as approximate.

Files changed 133 (51 headers in include/, 47 sources in src/, 32 tests, 3 other)
Diff +1331 / -2094 lines
Total comments/docstrings reviewed all of madspace/ except the vendored mixmax.hpp, dlpack.h, umami.h
Factual errors fixed, pass 1 roughly 100 (about 60 wrong descs in instruction_set.yaml, the rest spread over headers, kernels and tests)
Factual errors fixed, pass 2 about 40 edits (about 25 distinct errors; the weight vs det output name alone occurred in 17 headers)
In-function wall-of-text comments shortened to 1-3 lines roughly 200
History-style wording / commented-out code removed several dozen occurrences

Examples of errors that were fixed:

  • Mapping docs said every mapping returns a weight; the output is named det (17 headers).
  • scale_* instruction descs said "final-state momenta"; the kernels need the external momenta with the two incoming ones first.
  • Arcsine-map comment had the angle relation backwards (u = cos^2(phi/2)).
  • fixed_cwnet_fraction froze the channel-weight network at the end of training, not the start; batch_size_offset/batch_size_per_channel feed the total batch size, not each channel.
  • Context::load_matrix_element documented caching that does not exist; available_backends listed GPU backends it never returns.
  • Invariant documented a default power of 0.8 (it is 0) and an auxiliary mass that the kernels do not use.
  • pop, cat, squeeze, obs_sqrt_s, obs_eta, unweight, vegas_histogram, compute_scale etc. described behaviour that differed from the kernel.
  • install.py described an env-var override for cmake discovery that nothing reads.

Not fixed (needs a decision / code change)

Comments that cannot be checked from this repo (madevent Fortran, AmpliCol line references, paper section numbers) were left as written.

Possible code bugs noticed by the auditors (not touched here):

  • DiscreteOptimizer::optimize never writes the refined probabilities back into prob (the update loop is commented out), contrary to its docstring.
  • op_random_int allocates a float tensor but writes it through an int view; op_histogram / op_vegas_histogram truncate the bin index toward zero, so a value just below min lands in a bin instead of underflow/skip.
  • Luminosity reads result["s"]/result["r"], but Invariant outputs "invariant"/"random".
  • MatrixElementApi::device() can be uninitialised for matrix elements added through Context::add_matrix_element.
  • MadnisTraining::update_history wraps its history index at log_interval instead of integration_history_length.
  • VegasGridOptimizer.add_data binding expects different shapes/dtypes than the C++ implementation.
  • Several py::arg names in madspace.cpp differ from the C++ parameter names (e.g. NamedVector::push_back, save_globals/load_globals, PrettyBox), so the generated docstrings name the wrong parameter.
  • tests/test_mlm.py::test_extreme_kinematics_do_not_break_the_walk: docstring mentions asymmetric beams, the test samples symmetric kinematics.

Testing

  • python madspace/install.py --source -y builds.
  • pytest madspace/tests (run from outside the repo): 2008 passed.

Commits were made with --no-verify: the repo's clang-format hook reformats pre-existing, unformatted code, which would have buried the comment changes in formatting noise.

Fix comments and docstrings that disagreed with the code (headers, kernels, bindings, instruction_set.yaml descs, tests). Shorten in-function comments, drop wording that refers to earlier versions, remove commented-out code. Comments only; no code changes.
Re-verified all comments against the code; edits only where a comment was wrong (mapping output name det, scale_* momenta input, arcsine relation, backend/format/generator docs, and similar). Comments only.

This branch has not been deployed

No deployments
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.

1 participant