Repository navigation
refactor: store the SScalar derivatives sorted instead of in a hash map - #85
Merged
Merged
Conversation
A multiply of two single-entry values cost six allocations, eval copied the
name on every iteration, and binary and ternary copied the whole map. The
storage becomes a vector of name and derivative sorted by name, so combining
two values is a linear merge rather than a sequence of hash lookups and node
allocations. Measured on an expression chain, -O3:
names before after
2 416 ns, 12 allocs 86 ns, 2 allocs 4.8x
4 2029 ns, 53 allocs 280 ns, 6 allocs 7.2x
12 14271 ns, 375 allocs 1546 ns, 22 allocs 9.2x
This replaces the design proposed for this step. Interning the names behind a
shared table was going to be the approach, and the allocation accounting did
not survive scrutiny: with different name sets the merge and the remap of both
operands cost no less than the map, and in (x*y)/(x-y) the tables are equal in
content but not in identity, so the fast path would never have fired. Both
designs were prototyped and measured before choosing.
The sorted vector is not a detour for second order either. The position of a
name in the vector is its index, which is what a dense triangular Hessian would
be laid out over, and the merge already produces the union in sorted order.
Data stays the map. It is the interface type for construction and eval, which
is what callers and in particular a Python dict naturally provide; the vector
is internal. A second constructor taking the internal type would have made
SScalar(f, {{"x", 1.0}}) ambiguous between the two, so the operations go through
a named factory instead.
Three simplifications fall out. The four in-place operators delegate to their
binary form, which puts the merge in one place and makes aliased operands
correct without the guards added in #76. The derivatives are now iterated in a
defined order, so operator<< and repr are deterministic instead of unspecified
-- the tests and the README are tightened accordingly. And clang-tidy drops
from 50 findings to 41, because the braceless single statements it complained
about were in the code that went away.
Also replaces the int coefficients in operator+ and operator- with Scalar,
which matters for a float scalar type.
The public API is unchanged: 35 bound methods as before, SScalar(f=, d={...}),
size, d and eval identical. That the characterization from #84 carries the
rewrite without a single expected value being touched is the evidence that
doing it first was right.
C++ 112 test cases, 1421 assertions, Debug and Release with -Werror and
under clang with ASan+UBSan
Python 248 passed
clang-format and ruff format clean
oberbichler
force-pushed
the
refactor/sscalar-sorted-storage
branch
from
July 27, 2026 16:27
bf1aca0 to
9c09e19
Compare
This was referenced Jul 27, 2026
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.
Problem
SScalarstored its derivatives in anunordered_map<std::string, T>. A multiply of two single-entry values cost six allocations,evalcopied the name on every iteration, andbinary/ternarycopied the whole map for every operation.Change
The storage becomes a vector of name and derivative sorted by name, so combining two values is a linear merge rather than a sequence of hash lookups and node allocations. Measured on an expression chain,
-O3:This is not the design I proposed for this step
I had proposed interning the names behind a shared table, with the derivatives dense underneath. The allocation accounting did not survive scrutiny: with different name sets, merging the tables and remapping both operands costs no less than the map did — and in
(x*y)/(x-y)the two tables are equal in content but not in identity, so the pointer-comparison fast path the whole design rested on would never have fired.Both designs were prototyped and measured before choosing. The sorted vector wins at a fraction of the complexity: no name table, no
shared_ptr, no remapping, no identity subtleties.It is also not a detour for second order. The position of a name in the vector is its index — which is exactly what a dense triangular Hessian gets laid out over — and the merge already produces the union in sorted order.
API is unchanged
Datastays the map. It is the interface type for construction andeval, which is what callers — and in particular a Python dict — naturally provide; the sorted vector is internal.One trap on the way: a second constructor taking the internal type made
SScalar(f, {{"x", 1.0}})ambiguous betweenDataand the internal type. The operations go through a named factory instead of a constructor.Verified from Python: 35 bound methods as before,
SScalar(f=, d={...}),size,d()andevalall identical.Three simplifications fall out
operator<<andreprare deterministic instead of unspecified. test: characterize SScalar and document what it is #84 documented the order as unspecified; that is no longer true, and the tests and README are tightened to assert the sorted form.Also replaces the
intcoefficients inoperator+andoperator-withScalar, which matters for afloatscalar type.Verification
-Werror, and under clang with ASan+UBSanclang-format --Werrorandruff format --checkcleanThat the characterization from #84 carries this rewrite without a single expected value being touched is the evidence that doing it first was right.
Next
Step 3 is second order — named Hessians, dense triangular over the sorted names,
SSScalarin Python. The naming follows the rule the existing names actually encode: the count of leading letters is the order, asDScalar/DDScalarand the author's own commented-outD3SScalar/DD3SScalarshow.