test: cover all four order and sizing variants - #79
Merged
Merged
Conversation
The C++ suite ran almost entirely on DDScalar<2, double, 3>. Templates that are never instantiated are never compiled, so neither the tests, nor the warnings, nor the sanitizer job from #75 could see the first-order or dynamic code paths at all. That is how the deduction failure in #78 survived, and why turning on -Wall -Wextra produced zero warnings over a header that had five. Adds a second layer in its own translation unit: 14 TEST_CASE_TEMPLATEs over {order 1, 2} x {static, dynamic}, covering accessors and the linear Hessian index, the factories, all arithmetic in both binary and in-place form with both operand orders, one function per arity (unary, binary, ternary), eval, and padding. 56 test cases, 853 assertions. The expected values were derived symbolically from quadratic Taylor polynomials, so they do not share formulas with the header. One set of numbers serves both orders: the first-order part of every operation depends only on the first-order parts of its inputs, so truncating a second-order expectation to [f, g...] gives the first-order one -- the same trick the Python tests use. These pass on the first run, which on its own proves nothing, so they were validated by mutation. Two deliberate faults, counting failures in the old suite against the new one: unary() skips the gradient loop at order 1 old 0 new 4 data_length() one short for dynamic old 1 new 22 The first is invisible to the old suite. It would break every math function for every first-order type and still ship green. Kept separate from test.cpp rather than converting it: that file holds hand-verified numbers for one variant in full detail, which is worth keeping as-is, and a separate TU compiles in parallel (1.8 s next to 3.2 s). No production changes. Verified in Debug, Release and under clang with ASan+UBSan, all with -Werror: 101 test cases, 1302 assertions. Python 172 passed, unchanged with the file stashed.
Merged
oberbichler
added a commit
that referenced
this pull request
Jul 26, 2026
clang-format was already declared, in the dev group. clang-tidy was not available at all: not on PATH, no brew llvm, and the Xcode command line tools do not ship it. It goes into a group of its own rather than into dev, because the wheel is 44 MB against 1.8 MB for clang-format (115 MB against 26 MB unpacked, measured in fresh isolated environments). In dev, the format CI job would pull it on every run without using it. Two things were needed to make it actually run on this project: A compilation database. The test build is the right entry point, since that is what instantiates the templates -- an uninstantiated template is never analysed, which is the same reason the warnings from #75 saw nothing until #79 added the missing instantiations. The macOS SDK. CMake omits -isysroot because AppleClang finds the SDK implicitly; the standalone clang-tidy does not, and failed on signal.h from doctest, leaving the translation unit half parsed. CMAKE_OSX_SYSROOT puts the sysroot into the database, after which the unit parses with zero errors. The check selection is measured rather than guessed. Everything on produced 750+ findings, dominated by readability-identifier-length (427 -- i, j, da and daa are the notation of the domain) and modernize-use-trailing-return-type (225 -- a style the project does not use). The config turns the families on and names each exclusion with its reason, including two that are worth revisiting: modernize-use-nodiscard is off because annotating 57 members is an API decision rather than a lint fix, and bugprone-throwing-static-initialization only ever fires on the deliberate test fixtures. What remains is 51 findings and no errors. run-clang-tidy.py was tried and dropped: with two translation units it is no faster (1:56 against 1:58) and reports every finding in the header once per unit, 89 instead of 51. The justfile was untracked until now. It is included because the sysroot workaround and the compilation database step are not obvious enough to leave in one working copy. No source changes.
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.
Why
The C++ suite ran almost entirely on
DDScalar<2, double, 3>. Templates that are never instantiated are never compiled, so neither the tests, nor the warnings, nor the sanitizer job from #75 could see the first-order or dynamic code paths at all.That is not hypothetical. It is why the deduction failure in #78 survived — the only caller of that overload passed the size explicitly — and why turning on
-Wall -Wextrain #75 produced zero warnings over a header that had five.What
A second layer in its own translation unit: 14
TEST_CASE_TEMPLATEs over{order 1, 2} × {static, dynamic}— 56 test cases, 853 assertions.valuesf/g/haccessors, both index forms, setters — the linearh(i)is not exposed to Python, so this is its only coveragefactoriesempty,zero,constant,variable,variablesnegaddsubmuldivin place+=-=*=/=against the same expectations as the binary formsunary/binary/ternary functionssqrt,exp/atan2/hypot(a,b,c)— one per arity, since all the math functions funnel through those threeevalpaddingpad_left/pad_right, dynamic onlyThe expected values were derived symbolically from quadratic Taylor polynomials, so they share no formulas with the header. One set of numbers serves both orders: the first-order part of every operation depends only on the first-order parts of its inputs, so truncating a second-order expectation to
[f, g...]gives the first-order one — the same trick the Python tests already use inVariableSet.from_data.These tests passed on the first run
Which on its own proves nothing, so they were validated by mutation. Two deliberate faults in the header, counting failing test cases in the old suite against the new one:
unary()skips the gradient loop at order 1 (TOrder < 1→TOrder < 2)data_length()one short for the dynamic variantThe first is completely invisible to the old suite. It breaks every math function for every first-order type —
sqrt,exp, all 20 of them — and would ship green.Verification
-WerrorOne caveat worth recording: on first run the Python suite showed 4 failures, which turned out to be a stale extension build from #78's branch predating #76's aliasing fix — not a regression. After
uv sync --reinstall-package hyperjet, 172 passed.Notes
No production changes. Test-only.
Kept separate from
test.cpprather than converting it. That file holds hand-verified numbers for one variant in full detail, which is worth keeping as-is, and a separate TU compiles in parallel (1.8 s next to 3.2 s). The cost is some overlap between the two layers for theDDScalar<2, double, 3>case; consolidating is a later call.Not covered here, still open from the review:
SScalarhas no cross-variant axis to parametrize over, but its Python coverage is thin (init,mul, and the self-op tests from fix: handle aliased operands in the in-place operators #76) — no division, trigonometry, or comparisons.