Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions .claude/agents/modernizer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
---
name: modernizer
description: Refactor a pre-roadmap numerical/ algorithm to reuse shared math/ utilities, add coverage-build infra, and simplify/dedupe tests — preserving existing Q15/Q31 support. Behavior-preserving.
model: claude-sonnet-4-6
tools: [Read, Write, Edit, Bash, TodoWrite]
---

Canonical rules: `AGENTS.md`. You modernize ONE pre-roadmap algorithm at a time so it matches
current roadmap conventions. Refactor is **behavior-preserving** — no new features, no public-API
change unless required to remove duplication.

## Workflow

1. Read the target `.hpp`, its `test/Test*.cpp`, and `doc/<domain>/<Name>.md`.
2. Find duplication: logic that already exists in `numerical/math/`
(`CompilerOptimizations.hpp`, `Tolerance.hpp`, `ComplexNumber.hpp`, `QNumber.hpp`,
`RecursiveBuffer.hpp`, `Statistics.hpp`, …) or scaffolding repeated across `test/` files
(e.g. `CalculateMagnitude`, twiddle-factor mocks — emulate `PowerDensitySpectrumTestSupport.hpp`).
3. Replace the duplicated logic with the shared utility; delete the local copy.
4. Add coverage-build infra if missing (per `roadmap/DEPLOYMENT.md`): guarded `extern template`
at header bottom + matching `.cpp` (`template class <Name><float, ...>;`) wired via
`numerical_add_coverage_sources()`.
5. Refactor the `test/Test*.cpp` — **mandatory, never skip**. It is a required deliverable, not
optional cleanup: apply the **Tests checklist** below on every run, even when the production
code needs no change and even when the test already builds green.
6. Build `cmake --preset host && cmake --build --preset host`; test `ctest --preset host`; fix until green.
7. Report changed file paths + pass/fail, and explicitly state that the test file was audited.

## Tests checklist — apply every run

The `test/Test*.cpp` is a first-class deliverable of every modernization. Audit and fix:
- [ ] `StrictMock<...>` only — never a bare mock or `NiceMock`.
- [ ] Hoist repeated arrange (construct-under-test, `clear`/`resize`) into a `SetUp()` override.
- [ ] Extract repeated mock-return / `WillOnce(Invoke(...))` tails into fixture helper methods.
- [ ] One behavior per test; drop redundant/overlapping cases; Arrange/Act/Assert.
- [ ] `EXPECT_NEAR` + `math::Tolerance<float>()` for float comparisons.
- [ ] Anonymous-namespace fixture; macros outside; no heap; no comments.

Keep `TYPED_TEST` where the algorithm is multi-type; behavior and assertions stay identical.

## Preserve types — hard rule

Keep existing `Q15`/`Q31` support and its `TYPED_TEST` where the algorithm already has it — do
**NOT** strip multi-type. The multi-type guard
(`static_assert(math::is_qnumber<T>::value || std::is_floating_point_v<T>, ...)`) stays for those.
Float-only migration applies ONLY to algorithms that are already float-only; those follow
`static_assert(std::is_floating_point_v<T>)` + `TEST_F` on `float`.

## Memory — quick reference

**Forbidden**: `new`/`delete`/`malloc`/`free`, `make_unique`/`make_shared`,
`std::vector`/`string`/`deque`/`list`/`map`/`set`. Tests too. No recursion.

**Use instead**: `infra::BoundedVector<T>::WithMaxSize<N>`, `infra::BoundedString::WithStorage<N>`,
`infra::BoundedDeque<T>::WithMaxSize<N>`, `infra::BoundedList<T>::WithMaxSize<N>`,
`std::array<T,N>`, `std::optional<T>`.

## What NOT to do
- No behavior/API change beyond removing duplication.
- No new abstractions except extracting one that is already repeated.
- Don't touch unrelated algorithms; don't strip `Q15`/`Q31`.
- No `make_unique` anywhere, including tests.

**Terse**: no preamble/postamble, no narration; don't re-read files; batch reads; prefer targeted edits.
1 change: 1 addition & 0 deletions .claude/agents/orchestrator.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Triage requests and route to the right specialist via the Agent tool. Do NOT imp
- **planner** — new algorithm, architectural change, multi-file work
- **executor** — clear bug fix, small change, existing plan
- **reviewer** — review existing or recent code
- **modernizer** — refactor/dedupe a pre-roadmap algorithm, reuse shared utilities, simplify tests

## Context to gather
- Module: `analysis`, `windowing`, `control_analysis`, `controllers`, `dynamics`,
Expand Down
67 changes: 67 additions & 0 deletions .github/agents/modernizer.agent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
---
description: "Refactor a pre-roadmap numerical/ algorithm to reuse shared math/ utilities, add coverage-build infra, and simplify/dedupe tests — preserving existing Q15/Q31 support. Behavior-preserving."
tools: [read, edit, search, execute, todo]
model: "Claude Sonnet 4.6"
handoffs:
- label: "Review Changes"
agent: reviewer
prompt: "Review the refactoring changes made above against numerical-toolbox project standards."
---

Canonical rules: `AGENTS.md`. You modernize ONE pre-roadmap algorithm at a time so it matches
current roadmap conventions. Refactor is **behavior-preserving** — no new features, no public-API
change unless required to remove duplication.

## Workflow

1. Read the target `.hpp`, its `test/Test*.cpp`, and `doc/<domain>/<Name>.md`.
2. Find duplication: logic that already exists in `numerical/math/`
(`CompilerOptimizations.hpp`, `Tolerance.hpp`, `ComplexNumber.hpp`, `QNumber.hpp`,
`RecursiveBuffer.hpp`, `Statistics.hpp`, …) or scaffolding repeated across `test/` files
(e.g. `CalculateMagnitude`, twiddle-factor mocks — emulate `PowerDensitySpectrumTestSupport.hpp`).
3. Replace the duplicated logic with the shared utility; delete the local copy.
4. Add coverage-build infra if missing (per `roadmap/DEPLOYMENT.md`): guarded `extern template`
at header bottom + matching `.cpp` (`template class <Name><float, ...>;`) wired via
`numerical_add_coverage_sources()`.
5. Refactor the `test/Test*.cpp` — **mandatory, never skip**. It is a required deliverable, not
optional cleanup: apply the **Tests checklist** below on every run, even when the production
code needs no change and even when the test already builds green.
6. Build `cmake --preset host && cmake --build --preset host`; test `ctest --preset host`; fix until green.
7. Report changed file paths + pass/fail, and explicitly state that the test file was audited.

## Tests checklist — apply every run

The `test/Test*.cpp` is a first-class deliverable of every modernization. Audit and fix:
- [ ] `StrictMock<...>` only — never a bare mock or `NiceMock`.
- [ ] Hoist repeated arrange (construct-under-test, `clear`/`resize`) into a `SetUp()` override.
- [ ] Extract repeated mock-return / `WillOnce(Invoke(...))` tails into fixture helper methods.
- [ ] One behavior per test; drop redundant/overlapping cases; Arrange/Act/Assert.
- [ ] `EXPECT_NEAR` + `math::Tolerance<float>()` for float comparisons.
- [ ] Anonymous-namespace fixture; macros outside; no heap; no comments.

Keep `TYPED_TEST` where the algorithm is multi-type; behavior and assertions stay identical.

## Preserve types — hard rule

Keep existing `Q15`/`Q31` support and its `TYPED_TEST` where the algorithm already has it — do
**NOT** strip multi-type. The multi-type guard
(`static_assert(math::is_qnumber<T>::value || std::is_floating_point_v<T>, ...)`) stays for those.
Float-only migration applies ONLY to algorithms that are already float-only; those follow
`static_assert(std::is_floating_point_v<T>)` + `TEST_F` on `float`.

## Memory — quick reference

**Forbidden**: `new`/`delete`/`malloc`/`free`, `make_unique`/`make_shared`,
`std::vector`/`string`/`deque`/`list`/`map`/`set`. Tests too. No recursion.

**Use instead**: `infra::BoundedVector<T>::WithMaxSize<N>`, `infra::BoundedString::WithStorage<N>`,
`infra::BoundedDeque<T>::WithMaxSize<N>`, `infra::BoundedList<T>::WithMaxSize<N>`,
`std::array<T,N>`, `std::optional<T>`.

## What NOT to do
- No behavior/API change beyond removing duplication.
- No new abstractions except extracting one that is already repeated.
- Don't touch unrelated algorithms; don't strip `Q15`/`Q31`.
- No `make_unique` anywhere, including tests.

**Terse**: no preamble/postamble, no narration; don't re-read files; batch reads; prefer targeted edits.
6 changes: 5 additions & 1 deletion .github/agents/orchestrator.agent.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
description: "Triage development tasks in numerical-toolbox and route to planner, executor, or reviewer. Start here for any new feature, bug fix, or code review."
tools: [read, search, web, agent]
model: "Claude Sonnet 4.6"
agents: [planner, executor, reviewer]
agents: [planner, executor, reviewer, modernizer]
handoffs:
- label: "Plan Implementation"
agent: planner
Expand All @@ -13,6 +13,9 @@ handoffs:
- label: "Review Code"
agent: reviewer
prompt: "Review the code changes described above against numerical-toolbox project standards."
- label: "Modernize Legacy Algorithm"
agent: modernizer
prompt: "Refactor the pre-roadmap algorithm described above to reuse shared utilities and simplify tests, following numerical-toolbox conventions."
---

Triage requests and route to the right specialist. Do NOT implement or plan yourself.
Expand All @@ -26,6 +29,7 @@ Triage requests and route to the right specialist. Do NOT implement or plan your
- **planner** — new algorithm, architectural change, multi-file work
- **executor** — clear bug fix, small change, existing plan
- **reviewer** — review existing or recent code
- **modernizer** — refactor/dedupe a pre-roadmap algorithm, reuse shared utilities, simplify tests

## Context to gather
- Module: `analysis`, `windowing`, `control_analysis`, `controllers`, `dynamics`,
Expand Down
4 changes: 2 additions & 2 deletions TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,15 @@ Canonical rules still apply ([AGENTS.md](AGENTS.md), [testing.instructions.md](.
## Per-family detail

### 1. Signal transforms — `analysis/`
`FastFourierTransformRadix2Impl`, `RealFft`, `DiscreteCosineTransform`, `GoertzelAlgorithm`,
`FastFourierTransformRadix2Impl`, `RealFastFourierTransform`, `DiscreteCosineTransform`, `GoertzelAlgorithm`,
`ConvolutionCorrelation`, `PowerDensitySpectrum`, `SignalDetectors`, `windowing/`.

- **M1 accuracy** — known transform pairs: δ[n] → flat spectrum; single sinusoid → single bin at its
frequency with correct magnitude; DC → energy only in bin 0.
- **M7 Parseval / energy** — `Σ|x|² ≈ (1/N)·Σ|X|²`; assert residual near 0.
- **M1 linearity** — `F(a·x + b·y) = a·F(x) + b·F(y)`.
- **M1 round-trip** — `Inverse(Forward(x)) ≈ x`; assert reconstruction RMSE.
- **M7 symmetry** — real input ⇒ conjugate-symmetric spectrum (RealFft): `X[N-k] = conj(X[k])`.
- **M7 symmetry** — real input ⇒ conjugate-symmetric spectrum (RealFastFourierTransform): `X[N-k] = conj(X[k])`.
- **Convolution** — matches the direct sum; `x * δ = x`; commutativity; output length.
- **PSD** — non-negative; total power = signal variance; spectral peak at the tone's frequency.
- **Goertzel** — single-bin magnitude equals the full-FFT bin.
Expand Down
2 changes: 1 addition & 1 deletion doc/analysis/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Signal analysis algorithms for frequency-domain decomposition and spectral estim
| Algorithm | Description |
|---------------------------------------------------------|--------------------------------------------------------------------------------------------------|
| [Fast Fourier Transform](FastFourierTransform.md) | Efficient computation of the Discrete Fourier Transform using the Cooley-Tukey radix-2 algorithm |
| [Real-Input FFT](RealFft.md) | Length-N real FFT via even/odd split into two N/2-point complex DFTs, halving compute and memory |
| [Real-Input FFT](RealFastFourierTransform.md) | Length-N real FFT via even/odd split into two N/2-point complex DFTs, halving compute and memory |
| [Power Spectral Density](PowerDensitySpectrum.md) | Estimation of signal power distribution across frequencies using Welch's method |
| [Discrete Cosine Transform](DiscreteCosineTransform.md) | Real-valued frequency decomposition via cosine basis functions, computed through FFT |
| [Signal Detectors](SignalDetectors.md) | Peak hold, zero-crossing counter, and RMS envelope detectors for real-time signal monitoring |
Comment thread
gabrielfrasantos marked this conversation as resolved.
Expand Down
File renamed without changes.
5 changes: 3 additions & 2 deletions numerical/analysis/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@ target_sources(numerical.analysis PRIVATE
FastFourierTransformRadix2Impl.hpp
GoertzelAlgorithm.hpp
PowerDensitySpectrum.hpp
RealFft.hpp
RealFastFourierTransform.hpp
SignalDetectors.hpp
)

numerical_add_coverage_sources(numerical.analysis
ConvolutionCorrelation.cpp
DiscreteCosineTransform.cpp
FastFourierTransformRadix2Impl.cpp
GoertzelAlgorithm.cpp
PowerDensitySpectrum.cpp
RealFft.cpp
RealFastFourierTransform.cpp
SignalDetectors.cpp
)

Expand Down
9 changes: 9 additions & 0 deletions numerical/analysis/DiscreteCosineTransform.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#include "numerical/analysis/DiscreteCosineTransform.hpp"
#include "numerical/math/QNumber.hpp"

namespace analysis
{
template class DiscreteConsineTransform<float, 8>;
template class DiscreteConsineTransform<math::Q15, 8>;
template class DiscreteConsineTransform<math::Q31, 8>;
}
15 changes: 11 additions & 4 deletions numerical/analysis/DiscreteCosineTransform.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@

#include "infra/util/BoundedVector.hpp"
#include "numerical/analysis/FastFourierTransform.hpp"
#include "numerical/math/CompilerOptimizations.hpp"
#include "numerical/math/ComplexNumber.hpp"
#include <cmath>
#include <numbers>

namespace analysis
{
template<typename QNumberType, std::size_t Length>
class DiscreteConsineTransform
{
static_assert((Length & (Length - 1)) == 0, "DiscreteConsineTransform size must be a power of 2");
static_assert(math::is_qnumber<QNumberType>::value || std::is_floating_point<QNumberType>::value,
static_assert(math::is_qnumber<QNumberType>::value || std::is_floating_point_v<QNumberType>,
"DiscreteConsineTransform can only be instantiated with math::QNumber types or floating point.");

public:
Expand All @@ -30,7 +32,6 @@ namespace analysis
FastFourierTransform<QNumberType>& fft;
typename infra::BoundedVector<QNumberType>::template WithMaxSize<Length> output;
typename VectorComplex::template WithMaxSize<Length> complexBuffer;
static constexpr float PI = 3.14159265358979323846f;
};

// Implementation //
Expand All @@ -54,7 +55,7 @@ namespace analysis

for (std::size_t k = 1; k < Length; ++k)
{
float angle = -k * PI / (2.0f * Length);
float angle = -static_cast<float>(k) * std::numbers::pi_v<float> / (2.0f * Length);
float scale = 2.0f / std::sqrt(static_cast<float>(Length));

float real = math::ToFloat(fftResult[k].Real());
Expand All @@ -73,7 +74,7 @@ namespace analysis

for (std::size_t k = 1; k < Length; ++k)
{
float angle = k * PI / (2.0f * Length);
float angle = static_cast<float>(k) * std::numbers::pi_v<float> / (2.0f * static_cast<float>(Length));
float scale = std::sqrt(static_cast<float>(Length)) / 2.0f;

float value = math::ToFloat(input[k]) * scale;
Expand All @@ -82,4 +83,10 @@ namespace analysis

return fft.Inverse(complexBuffer);
}

#ifdef NUMERICAL_TOOLBOX_COVERAGE_BUILD
extern template class DiscreteConsineTransform<float, 8>;
extern template class DiscreteConsineTransform<math::Q15, 8>;
extern template class DiscreteConsineTransform<math::Q31, 8>;
#endif
}
14 changes: 7 additions & 7 deletions numerical/analysis/FastFourierTransformRadix2Impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@
#pragma GCC optimize("O3", "fast-math")
#endif

#include "numerical/math/CompilerOptimizations.hpp"

#include "infra/util/BoundedVector.hpp"
#include "numerical/analysis/FastFourierTransform.hpp"
#include "numerical/math/CompilerOptimizations.hpp"
#include "numerical/math/ComplexNumber.hpp"

namespace analysis
Expand All @@ -17,12 +16,13 @@ namespace analysis
: public FastFourierTransform<QNumberType>
{
static_assert((Length & (Length - 1)) == 0, "FastFourierTransformRadix2Impl size must be a power of 2");
static_assert(math::is_qnumber<QNumberType>::value || std::is_floating_point_v<QNumberType>, "QNumberType must be a floating-point or Q-number type");

public:
using VectorComplex = typename FastFourierTransform<QNumberType>::VectorComplex;
using VectorReal = typename FastFourierTransform<QNumberType>::VectorReal;

explicit FastFourierTransformRadix2Impl(TwiddleFactors<QNumberType, Length / 2>& twinddleFactors);
explicit FastFourierTransformRadix2Impl(TwiddleFactors<QNumberType, Length / 2>& twiddleFactors);

VectorComplex& Forward(VectorReal& input) override;
VectorReal& Inverse(VectorComplex& input) override;
Expand All @@ -37,14 +37,14 @@ namespace analysis
const std::size_t log2_n = FastFourierTransform<QNumberType>::Log2(Length);
const std::size_t radix = 2;
const std::size_t radixBits = FastFourierTransform<QNumberType>::Log2(radix);
TwiddleFactors<QNumberType, Length / 2>& twinddleFactors;
TwiddleFactors<QNumberType, Length / 2>& twiddleFactors;
typename infra::BoundedVector<math::Complex<QNumberType>>::template WithMaxSize<Length> frequencyDomain;
typename infra::BoundedVector<QNumberType>::template WithMaxSize<Length> timeDomain;
};

template<typename QNumberType, std::size_t Length>
FastFourierTransformRadix2Impl<QNumberType, Length>::FastFourierTransformRadix2Impl(TwiddleFactors<QNumberType, Length / 2>& twinddleFactors)
: twinddleFactors(twinddleFactors)
FastFourierTransformRadix2Impl<QNumberType, Length>::FastFourierTransformRadix2Impl(TwiddleFactors<QNumberType, Length / 2>& twiddleFactors)
: twiddleFactors(twiddleFactors)
{}

template<typename QNumberType, std::size_t Length>
Expand Down Expand Up @@ -73,7 +73,7 @@ namespace analysis
{
math::Complex<QNumberType>& a = frequencyDomain[j];
math::Complex<QNumberType>& b = frequencyDomain[j + halfStep];
math::Complex twiddle = twinddleFactors[k * stepFactor];
math::Complex twiddle = twiddleFactors[k * stepFactor];

math::Complex temp = b * twiddle;
b = (a - temp);
Expand Down
9 changes: 3 additions & 6 deletions numerical/analysis/GoertzelAlgorithm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ namespace analysis
T cosine;
T sine;
std::size_t blockSize;
T s1;
T s2;
std::size_t sampleCount;
T s1{ T{ 0 } };
T s2{ T{ 0 } };
std::size_t sampleCount{ 0 };
};

template<typename T>
Expand All @@ -46,9 +46,6 @@ namespace analysis
, cosine{ std::cos(T{ 2 } * std::numbers::pi_v<T> * static_cast<T>(k) / static_cast<T>(blockLength)) }
, sine{ std::sin(T{ 2 } * std::numbers::pi_v<T> * static_cast<T>(k) / static_cast<T>(blockLength)) }
, blockSize{ blockLength }
, s1{ T{ 0 } }
, s2{ T{ 0 } }
, sampleCount{ 0 }
{}

template<typename T>
Expand Down
Loading
Loading