docs: full review + reflect performance optimizations and API changes#184
Merged
Conversation
Reconcile docs against current master after recent performance and API changes. Each edit is grounded in the current source; capabilities that exist only on unmerged feature branches (P8 TriMultiply fusion, -ffp-contract=fast) are deliberately not documented, since docs describe the single current version. - random.md: document the Sobol `polish_` member and the 5-arg `NewSobol` / `SobolRSG_` constructors (polish defaults to false, halving per-deviate InverseNCDF cost at unchanged QMC accuracy). - pde.md: document the cached implicit-operator decomposition in FD1D_ (cachedDecomp_, CacheHit, DecompositionsSinceInit) reused across time-homogeneous rolls. - yield_curve.md, yield_curve_jacobian.md, and the AAD analytic-Jacobian experimental note: correct the recording contract to start with `Rewind` rather than `Clear`. TapeGuard_ calls Dal::AAD::Rewind on construction and destruction (not Clear), matching the aad.md methodology and the actual calibration / joint-calibration sources. Index entries in docs/README.md updated for the new random.md and pde.md content. No CHANGELOG entry: this is a docs reconciliation pass, below the fundamental-change bar. Co-Authored-By: Claude <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
Coverage Report for CI Build 28320955718Coverage decreased (-0.004%) to 81.491%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions67 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Pull request overview
Docs-only reconciliation pass across DAL’s methodology notes to reflect current master behavior after recent AAD API and performance-related changes (notably Rewind vs Clear, Sobol normal-draw knobs, and FD1D_ decomposition caching), plus index updates.
Changes:
- Updated AAD recording/RAII wording in yield-curve docs to use
Rewindinstead ofClear. - Added/expanded methodology details for Sobol normal-draw settings (
polish_,precise_) andFD1D_cached implicit decomposition behavior. - Updated
docs/README.mdmethodology bullets to reflect the above additions.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/README.md | Updates methodology index bullets for PDE caching and Sobol normal-draw knobs. |
| docs/methodology/yield_curve.md | Rewords TapeGuard_/recording-contract documentation to use Rewind and explain allocation reuse vs Clear. |
| docs/methodology/yield_curve_jacobian.md | Updates joint calibration TapeGuard wording (clears → rewinds). |
| docs/methodology/random.md | Adds a Sobol “Normal-Draw Precision and polish_” section documenting constructor surfaces and persistence. |
| docs/methodology/pde.md | Documents FD1D_ cached implicit-operator decomposition and cache-hit conditions. |
| docs/experimental/aad-analytic-jacobian-curve-calibration.md | Updates the backend-neutral recording contract to start with Rewind(*Tape()). |
Comment on lines
137
to
139
| no fallback. `CalibrateJointMultiCurve` is single-threaded: the AAD tape is | ||
| thread-local and a `TapeGuard_` clears it on entry and exit (also under | ||
| thread-local and a `TapeGuard_` rewinds it on entry and exit (also under | ||
| exception unwind), so concurrent calls would corrupt the tape. |
Comment on lines
+193
to
+194
| - `precise_` selects a higher-accuracy inverse-normal-CDF routine, mirroring the | ||
| pseudo-random family. |
Comment on lines
108
to
111
| - Brownian bridge: bisection order, conditional mean/variance, variation normalization | ||
| - Sobol direction numbers and the Gray-code $O(1)$ recurrence | ||
| - Sobol normal-draw precision knobs: `precise_` (high-accuracy inverse CDF) and `polish_` (Newton polish, default off) | ||
| - Path seeking via direct state reconstruction (`SobolSet_::SkipTo`, MRG32k32a matrix jump) |
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
Full review of
docs/for structure, conciseness, readability, formatting, and cross-references, plus reconciliation against the currentmastersource after recent performance and API changes. Every edit is grounded in the current source.Documented (confirmed on
master):docs/methodology/random.md— Sobolpolish_member and the 5-argNewSobol/SobolRSG_constructors (polishdefaults tofalse, halving per-deviateInverseNCDFcost at unchanged QMC accuracy; persisted by the storable markup). Source:dal-cpp/dal/math/random/sobol.hpp,sobol.cpp.docs/methodology/pde.md—FD1D_cached implicit-operator decomposition (cachedDecomp_,CacheHit,DecompositionsSinceInit), reused across time-homogeneous rolls. Source:dal-cpp/dal/math/pde/fd1d.hpp,fd1d.cpp.Stale references fixed (
Clear→Rewind):docs/methodology/yield_curve.md—TapeGuard_description and the joint-path recording contract now correctly stateRewind.TapeGuard_callsDal::AAD::Rewindon construction and destruction (notClear); verified indal-cpp/dal/curve/tapeguard.hpp.docs/methodology/yield_curve_jacobian.md—TapeGuard_"rewinds" (was "clears").docs/experimental/aad-analytic-jacobian-curve-calibration.md— recording contract starts withRewind(*Tape()).Index sync:
docs/README.mdbullets forrandom.mdandpde.mdupdated.docs/README.mdandCLAUDE.mdmethodology lists are complete and in sync (all 14 methodology docs).Deliberately NOT documented (exist only on unmerged feature branches, not on
master):TriMultiplyneighbour-correction loop fusion — only onperf/p7-p8-fused-sweeps(commit61115fais not an ancestor ofmaster);masterships the original unfused three-passTriMultiply.matrix.mdis already correct.-ffp-contract=fast//fp:contractinPlatform.cmake— only onperf/enable-fp-contract-all-compilers.installation.mdis already correct.volatile/mutable/ no-large-comments style rules — not present in.claude/rules/code-style.md; no doc references them.No CHANGELOG entry — this is a docs polish/reconciliation pass, below the fundamental-change bar.
Test plan
No test suite applies to a docs-only change. Manual verification performed:
sobol.hpp,sobol.cpp,fd1d.hpp,fd1d.cpp,tapeguard.hpp,calibration.cpp,jointcalibration.cpp) to ground each edit.CurveSolverOptions_are onmaster; confirmed P8 fusion and fp-contract are NOT ancestors ofmaster.aad.mdRewind methodology against the updatedyield_curve.md/yield_curve_jacobian.md/ experimental note for vocabulary consistency.docs/README.mdandCLAUDE.mdmethodology lists match (14 docs each).$$) are balanced in the changed methodology docs..cpp/.hpp/.h) ordal-cpp/dal/auto/files were modified.Co-Authored-By: Claude noreply@anthropic.com