Repository navigation
madspace: audit comments and docstrings against the source - #364
Draft
theoheimel wants to merge 2 commits into
Draft
theoheimel wants to merge 2 commits into
theoheimel wants to merge 2 commits into
Conversation
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
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
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.
Summary
Systematic audit of the comments and docstrings in
madspace/(C++ headers and sources, kernels, Python bindings and loader, build scripts,instruction_set.yamldescriptions, 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.yamlis structurally identical with thedesckeys stripped.Two commits, one per pass:
Stats
Counts are tallied from the audit agents' reports, not from an exhaustive static count, so treat them as approximate.
include/, 47 sources insrc/, 32 tests, 3 other)madspace/except the vendoredmixmax.hpp,dlpack.h,umami.hinstruction_set.yaml, the rest spread over headers, kernels and tests)weightvsdetoutput name alone occurred in 17 headers)Examples of errors that were fixed:
weight; the output is nameddet(17 headers).scale_*instruction descs said "final-state momenta"; the kernels need the external momenta with the two incoming ones first.fixed_cwnet_fractionfroze the channel-weight network at the end of training, not the start;batch_size_offset/batch_size_per_channelfeed the total batch size, not each channel.Context::load_matrix_elementdocumented caching that does not exist;available_backendslisted GPU backends it never returns.Invariantdocumented a defaultpowerof 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_scaleetc. described behaviour that differed from the kernel.install.pydescribed 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::optimizenever writes the refined probabilities back intoprob(the update loop is commented out), contrary to its docstring.op_random_intallocates a float tensor but writes it through an int view;op_histogram/op_vegas_histogramtruncate the bin index toward zero, so a value just belowminlands in a bin instead of underflow/skip.Luminosityreadsresult["s"]/result["r"], butInvariantoutputs"invariant"/"random".MatrixElementApi::device()can be uninitialised for matrix elements added throughContext::add_matrix_element.MadnisTraining::update_historywraps its history index atlog_intervalinstead ofintegration_history_length.VegasGridOptimizer.add_databinding expects different shapes/dtypes than the C++ implementation.py::argnames inmadspace.cppdiffer 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 -ybuilds.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.