feat(fft): implement 1D FFT/IFFT and frequency helpers - #233
Open
sonusharma6-dsa wants to merge 8 commits into
Open
feat(fft): implement 1D FFT/IFFT and frequency helpers#233sonusharma6-dsa wants to merge 8 commits into
sonusharma6-dsa wants to merge 8 commits into
Conversation
Signed-off-by: sonusharma6-dsa <ramprashadkumar11@gmail.com> (cherry picked from commit 7bbd6db) Signed-off-by: sonusharma6-dsa <ramprashadkumar11@gmail.com>
Signed-off-by: sonusharma6-dsa <ramprashadkumar11@gmail.com>
Signed-off-by: sonusharma6-dsa <ramprashadkumar11@gmail.com>
…ssary casts) Signed-off-by: sonusharma6-dsa <ramprashadkumar11@gmail.com>
Signed-off-by: sonusharma6-dsa <ramprashadkumar11@gmail.com>
…and fft modules Signed-off-by: sonusharma6-dsa <ramprashadkumar11@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds 1-D FFT/IFFT and frequency-axis utilities to mohu-fft (fftfreq, rfftfreq, fftshift, ifftshift, fft, ifft), implements std::str::FromStr for DType, and applies minor formatting and float-literal adjustments across dtype and error crates. ChangesFFT Transform Implementation and Library Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Implements FFT/IFFT functionality and frequency helpers in the mohu-fft crate, adds a FromStr impl for DType, and applies formatting/cleanup changes across mohu-error and mohu-dtype.
Changes:
- Adds
fft/ifftimplementations (withNormscaling) andfftfreq/rfftfreq/fftshift/ifftshifthelpers, including basic tests. - Adds
std::str::FromStrimplementation forDType. - Minor formatting/cleanup updates in
mohu-errorandmohu-dtype.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/mohu-fft/src/transform.rs | New 1-D FFT/IFFT with normalization modes and roundtrip test. |
| crates/mohu-fft/src/freq.rs | New frequency-axis helpers (fftfreq, rfftfreq, fftshift, ifftshift) with tests. |
| crates/mohu-fft/Cargo.toml | Adds cargo-machete ignored-deps metadata. |
| crates/mohu-error/src/test_utils.rs | Import reordering and one-line formatting of helpers. |
| crates/mohu-error/src/reporter.rs | Whitespace/alignment cleanup in fmt_full. |
| crates/mohu-dtype/src/macros.rs | Removes a blank line between doc table and section comment. |
| crates/mohu-dtype/src/finfo.rs | Replaces (N_u32 as f64) with N_f64 literals in precision calculations. |
| crates/mohu-dtype/src/dtype.rs | Adds std::str::FromStr for DType delegating to DType::from_str. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+526
to
+532
| impl std::str::FromStr for DType { | ||
| type Err = MohuError; | ||
|
|
||
| fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
| DType::from_str(s) | ||
| } | ||
| } |
| } | ||
|
|
||
| /// Shift the zero-frequency component to the center of the spectrum. | ||
| pub fn fftshift<T: Clone>(mut v: Vec<T>) -> Vec<T> { |
Comment on lines
+67
to
+71
| #[test] | ||
| fn roundtrip_fft_ifft() { | ||
| let input: Vec<Complex<f64>> = (0..8).map(|i| Complex::new(i as f64, 0.0)).collect(); | ||
| let out = fft(&input, None, Norm::Backward); | ||
| let back = ifft(&out, None, Norm::Backward); |
Comment on lines
19
to
22
| /// | [`dispatch_signed!`] | signed integers + floats | | ||
| /// | [`for_each_dtype!`] | invoke a macro for every dtype (codegen helper) | | ||
| /// | [`assert_dtype!`] | assert a DType at runtime or return an error | | ||
|
|
||
| // ─── dtype_of! ─────────────────────────────────────────────────────────────── |
Author
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/mohu-fft/src/freq.rs`:
- Around line 23-24: The docstring for rfftfreq is inaccurate: update the
comment for the pub fn rfftfreq(n: usize, d: f64) -> Vec<f64> to state that it
returns only the non-negative frequency bins used for real-input (length = n/2 +
1) and that it differs from fftfreq by omitting negative frequencies (i.e., it
follows rfft conventions), and include the formula/units for d if present so
callers understand the bin spacing.
- Around line 34-35: Remove the unnecessary mut from the fftshift and ifftshift
function parameters (change signatures from e.g. pub fn fftshift<T: Clone>(mut
v: Vec<T>) to pub fn fftshift<T: Clone>(v: Vec<T>) and similarly for ifftshift)
to eliminate unused_mut warnings; in the tests replace the Vec<f64> equality
assertion in tests::test_fftfreq_len with moh u-testing::approx::assert_allclose
(use assert_allclose on the float vector instead of assert_eq!); and update the
rfftfreq doc comment to clearly state it returns only the non-negative frequency
bins (length n/2+1) rather than matching fftfreq output exactly.
- Around line 63-67: The test test_fftfreq_len currently uses assert_eq! to
compare the Vec<f64> returned by fftfreq, which is unsafe for floating-point
values; replace the assertion with mohu_testing::approx::assert_allclose! and
import it if needed, e.g. assert_allclose!(f, vec![0.0, 0.25, -0.5, -0.25], rtol
= 1e-5, atol = 1e-8); keep the test name test_fftfreq_len and the fftfreq call
unchanged—only change the assertion macro and add any required use statement for
mohu_testing::approx::assert_allclose!.
In `@crates/mohu-fft/src/transform.rs`:
- Around line 72-75: In tests::roundtrip_fft_ifft replace the two manual epsilon
asserts inside the for (a, b) in input.iter().zip(back.iter()) loop with a
single call to moh_testing::approx::assert_allclose so the project
float-comparison helper is used; compare the complex values from input and back
(a and b) with tolerance 1e-9 (or the helper’s equivalent args) by invoking
assert_allclose on the pair/values instead of using (a.re - b.re).abs() and
(a.im - b.im).abs() checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c8a6d083-7e39-40f9-8cdf-0c35ae57b3c8
📒 Files selected for processing (8)
crates/mohu-dtype/src/dtype.rscrates/mohu-dtype/src/finfo.rscrates/mohu-dtype/src/macros.rscrates/mohu-error/src/reporter.rscrates/mohu-error/src/test_utils.rscrates/mohu-fft/Cargo.tomlcrates/mohu-fft/src/freq.rscrates/mohu-fft/src/transform.rs
💤 Files with no reviewable changes (1)
- crates/mohu-dtype/src/macros.rs
Comment on lines
+23
to
+24
| /// Alias for `fftfreq` for real-input transforms; behavior is identical. | ||
| pub fn rfftfreq(n: usize, d: f64) -> Vec<f64> { |
There was a problem hiding this comment.
Fix rfftfreq doc wording; it is not identical to fftfreq.
The implementation returns only non-negative bins (n/2 + 1), so the current “behavior is identical” statement is misleading.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/mohu-fft/src/freq.rs` around lines 23 - 24, The docstring for rfftfreq
is inaccurate: update the comment for the pub fn rfftfreq(n: usize, d: f64) ->
Vec<f64> to state that it returns only the non-negative frequency bins used for
real-input (length = n/2 + 1) and that it differs from fftfreq by omitting
negative frequencies (i.e., it follows rfft conventions), and include the
formula/units for d if present so callers understand the bin spacing.
Comment on lines
+63
to
+67
| #[test] | ||
| fn test_fftfreq_len() { | ||
| let f = fftfreq(4, 1.0); | ||
| assert_eq!(f, vec![0.0, 0.25, -0.5, -0.25]); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and print the relevant section around the referenced lines.
FILE="crates/mohu-fft/src/freq.rs"
if [ ! -f "$FILE" ]; then
echo "Missing: $FILE"
exit 1
fi
echo "== $FILE =="
nl -ba "$FILE" | sed -n '40,110p'Repository: mohu-org/mohu
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="crates/mohu-fft/src/freq.rs"
echo "== File: $FILE =="
ls -la "$FILE" || true
echo
echo "== Lines 40-110 (approx) =="
# Use sed for approximate line ranges
sed -n '40,110p' "$FILE" | sed -e 's/\t/[TAB]/g'
echo
echo "== rg: fftfreq and assert_* usage =="
rg -n "fftfreq|assert_eq!|assert_allclose" "$FILE" || trueRepository: mohu-org/mohu
Length of output: 1515
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="crates/mohu-fft/src/freq.rs"
echo "== File: $FILE =="
ls -la "$FILE" || true
echo
echo "== Lines 40-110 (approx) =="
sed -n '40,110p' "$FILE" | sed -e 's/\t/[TAB]/g'
echo
echo "== rg: fftfreq and assert_* usage =="
rg -n "fftfreq|assert_eq!|assert_allclose" "$FILE" || trueRepository: mohu-org/mohu
Length of output: 1515
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "assert_allclose" -S . | head -n 50Repository: mohu-org/mohu
Length of output: 938
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "assert_allclose" -S . | head -n 50Repository: mohu-org/mohu
Length of output: 938
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="crates/mohu-testing/src/lib.rs"
echo "== head 120 =="
sed -n '1,140p' "$FILE" | sed -e 's/\t/[TAB]/g'
echo
echo "== rg assert_allclose! usage (first 30) =="
rg -n "assert_allclose!\s*\(" -S . | head -n 30Repository: mohu-org/mohu
Length of output: 1960
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="crates/mohu-testing/src/lib.rs"
echo "== crates/mohu-testing/src/lib.rs (top 180) =="
sed -n '1,180p' "$FILE" | sed -e 's/\t/[TAB]/g'
echo
echo "== First usages of assert_allclose!(...) =="
rg -n "assert_allclose!\s*\(" -S . | head -n 30Repository: mohu-org/mohu
Length of output: 1993
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="crates/mohu-testing/src/approx.rs"
if [ ! -f "$FILE" ]; then
# Try locating approx module file
echo "approx.rs not found, listing crates/mohu-testing/src:"
ls -ლა crates/mohu-testing/src || true
fi
echo "== approx module search =="
rg -n "macro_rules!\s*assert_allclose|assert_allclose!\s*\(|pub\s+fn\s+assert_allclose|macro\s+assert_allclose" crates/mohu-testing/src/approx.rs || true
echo
echo "== approx.rs head 220 =="
sed -n '1,220p' crates/mohu-testing/src/approx.rs | sed -e 's/\t/[TAB]/g'Repository: mohu-org/mohu
Length of output: 145
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== approx module search =="
rg -n "macro_rules!\s*assert_allclose|assert_allclose!\s*\(|pub\s+fn\s+assert_allclose|macro\s+assert_allclose" crates/mohu-testing/src/approx.rs || true
echo
echo "== crates/mohu-testing/src/approx.rs (top 260) =="
sed -n '1,260p' crates/mohu-testing/src/approx.rs | sed -e 's/\t/[TAB]/g'Repository: mohu-org/mohu
Length of output: 170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la crates/mohu-testing/src
echo "== wc -l approx.rs =="
wc -l crates/mohu-testing/src/approx.rs
echo
echo "== rg assert_allclose in mohu-testing/src =="
rg -n "assert_allclose" crates/mohu-testing/src -S || true
echo
echo "== rg macro_rules assert_allclose in repo =="
rg -n "macro_rules!\s*assert_allclose" -S . || trueRepository: mohu-org/mohu
Length of output: 1205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la crates/mohu-testing/src
echo "== wc -l approx.rs =="
wc -l crates/mohu-testing/src/approx.rs
echo
echo "== rg assert_allclose in mohu-testing/src =="
rg -n "assert_allclose" crates/mohu-testing/src -S || true
echo
echo "== rg macro_rules assert_allclose in repo =="
rg -n "macro_rules!\s*assert_allclose" -S . || trueRepository: mohu-org/mohu
Length of output: 1205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if [ -e crates/mohu-testing/src/approx ]; then
echo "== ls -la crates/mohu-testing/src/approx =="
ls -la crates/mohu-testing/src/approx
echo
echo "== Determine type =="
file crates/mohu-testing/src/approx || true
echo
echo "== rg assert_allclose in approx path =="
rg -n "assert_allclose" crates/mohu-testing/src/approx -S || true
echo
echo "== tree (shallow) for approx path =="
(command -v tree >/dev/null && tree -L 4 crates/mohu-testing/src/approx) || true
else
echo "Path not found: crates/mohu-testing/src/approx"
fiRepository: mohu-org/mohu
Length of output: 104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if [ -e crates/mohu-testing/src/approx ]; then
echo "== ls -la crates/mohu-testing/src/approx =="
ls -la crates/mohu-testing/src/approx
echo
echo "== Determine type =="
file crates/mohu-testing/src/approx || true
echo
echo "== rg assert_allclose in approx path =="
rg -n "assert_allclose" crates/mohu-testing/src/approx -S || true
echo
echo "== tree (shallow) for approx path =="
(command -v tree >/dev/null && tree -L 4 crates/mohu-testing/src/approx) || true
else
echo "Path not found: crates/mohu-testing/src/approx"
fiRepository: mohu-org/mohu
Length of output: 104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check approx path =="
ls -la crates/mohu-testing/src/approx || true
file crates/mohu-testing/src/approx || true
echo
echo "== Head approx (first 80 lines) =="
sed -n '1,80p' crates/mohu-testing/src/approx || true
echo
echo "== rg assert_allclose in crates/mohu-testing/src =="
rg -n "assert_allclose" crates/mohu-testing/src -S || trueRepository: mohu-org/mohu
Length of output: 766
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check approx path =="
ls -la crates/mohu-testing/src/approx || true
file crates/mohu-testing/src/approx || true
echo
echo "== Head approx (first 80 lines) =="
sed -n '1,80p' crates/mohu-testing/src/approx || true
echo
echo "== rg assert_allclose in crates/mohu-testing/src =="
rg -n "assert_allclose" crates/mohu-testing/src -S || trueRepository: mohu-org/mohu
Length of output: 766
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import os
p="crates/mohu-testing/src"
print("Exists:", os.path.exists(p))
print("Entries:")
for name in sorted(os.listdir(p)):
print("-",repr(name))
PY
echo
echo "== ls -la crates/mohu-testing/src =="
ls -la crates/mohu-testing/src
echo
for f in crates/mohu-testing/src/approx crates/mohu-testing/src/assert crates/mohu-testing/src/dtype; do
echo "== try head for $f =="
if [ -e "$f" ]; then
# show first 80 lines or cat if small
head -n 80 "$f" 2>/dev/null || true
else
echo "Missing: $f"
fi
done
echo
echo "== rg assert_allclose across repo (excluding target dirs, limit output) =="
rg -n "assert_allclose" -S . | head -n 100
echo
echo "== rg assert_allclose across mohu-testing/src (excluding lib.rs docs) =="
rg -n "assert_allclose" -S crates/mohu-testing/src --glob '!lib.rs'Repository: mohu-org/mohu
Length of output: 2180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import os
p="crates/mohu-testing/src"
print("Exists:", os.path.exists(p))
print("Entries:")
for name in sorted(os.listdir(p)):
print("-",repr(name))
PY
echo
echo "== ls -la crates/mohu-testing/src =="
ls -la crates/mohu-testing/src
echo
for f in crates/mohu-testing/src/approx crates/mohu-testing/src/assert crates/mohu-testing/src/dtype; do
echo "== try head for $f =="
if [ -e "$f" ]; then
head -n 120 "$f" 2>/dev/null || true
else
echo "Missing: $f"
fi
done
echo
echo "== rg assert_allclose across repo (first 120 matches) =="
rg -n "assert_allclose" -S . | head -n 120
echo
echo "== rg assert_allclose in crates/mohu-testing/src (excluding lib.rs) =="
rg -n "assert_allclose" -S crates/mohu-testing/src --glob '!lib.rs' || trueRepository: mohu-org/mohu
Length of output: 2160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== crates/mohu-testing/src/assert.rs =="
sed -n '1,220p' crates/mohu-testing/src/assert.rs | sed -e 's/\t/[TAB]/g'
echo
echo "== rg assert_allclose in assert.rs =="
rg -n "assert_allclose" crates/mohu-testing/src/assert.rs -S || true
echo
echo "== crates/mohu-testing/src/lib.rs (tail 120) =="
sed -n '120,260p' crates/mohu-testing/src/lib.rs | sed -e 's/\t/[TAB]/g'
echo
echo "== rg assert_allclose in lib.rs excluding docs =="
rg -n "assert_allclose" crates/mohu-testing/src/lib.rs -S || trueRepository: mohu-org/mohu
Length of output: 530
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== crates/mohu-testing/src/assert.rs =="
sed -n '1,220p' crates/mohu-testing/src/assert.rs | sed -e 's/\t/[TAB]/g'
echo
echo "== rg assert_allclose in assert.rs =="
rg -n "assert_allclose" crates/mohu-testing/src/assert.rs -S || true
echo
echo "== crates/mohu-testing/src/lib.rs (tail 120) =="
sed -n '120,260p' crates/mohu-testing/src/lib.rs | sed -e 's/\t/[TAB]/g'
echo
echo "== rg assert_allclose in lib.rs excluding docs =="
rg -n "assert_allclose" crates/mohu-testing/src/lib.rs -S || trueRepository: mohu-org/mohu
Length of output: 530
Use mohu_testing::approx::assert_allclose! instead of assert_eq! for Vec<f64> in fftfreq tests.
crates/mohu-fft/src/freq.rs (test_fftfreq_len) compares fftfreq(...)’s Vec<f64> with assert_eq!, which violates the float-comparison rule—switch to assert_allclose! (e.g., assert_allclose!(f, vec![0.0, 0.25, -0.5, -0.25], rtol = 1e-5, atol = 1e-8);).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/mohu-fft/src/freq.rs` around lines 63 - 67, The test test_fftfreq_len
currently uses assert_eq! to compare the Vec<f64> returned by fftfreq, which is
unsafe for floating-point values; replace the assertion with
mohu_testing::approx::assert_allclose! and import it if needed, e.g.
assert_allclose!(f, vec![0.0, 0.25, -0.5, -0.25], rtol = 1e-5, atol = 1e-8);
keep the test name test_fftfreq_len and the fftfreq call unchanged—only change
the assertion macro and add any required use statement for
mohu_testing::approx::assert_allclose!.
Comment on lines
+72
to
+75
| for (a, b) in input.iter().zip(back.iter()) { | ||
| assert!((a.re - b.re).abs() < 1e-9); | ||
| assert!((a.im - b.im).abs() < 1e-9); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant portion around the reported lines
echo "=== transform.rs lines 50-100 ==="
nl -ba crates/mohu-fft/src/transform.rs | sed -n '50,110p'
# Find other float-assert patterns in this file
echo "=== float epsilon asserts in transform.rs ==="
rg -n "abs\(\)\s*<\s*1e-9|1e-9|epsilon" crates/mohu-fft/src/transform.rs || true
# Check how/where assert_allclose is used elsewhere in the repo
echo "=== usages of mohu_testing::approx::assert_allclose ==="
rg -n "mohu_testing::approx::assert_allclose|assert_allclose\(" crates | head -n 50
# Check the test module location for compliance (cfg(test) at bottom)
echo "=== cfg(test) modules at bottom-ish ==="
rg -n "cfg\(test\)" crates/mohu-fft/src/transform.rs
nl -ba crates/mohu-fft/src/transform.rs | tail -n 80Repository: mohu-org/mohu
Length of output: 152
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify file exists
echo "=== file check ==="
ls -la crates/mohu-fft/src/transform.rs
# Show relevant portion around the reported lines (use awk for line numbers)
echo "=== transform.rs lines 50-110 ==="
awk 'NR>=50 && NR<=110 {printf "%d\t%s\n", NR, $0}' crates/mohu-fft/src/transform.rs
# Find float epsilon asserts in this file
echo "=== float epsilon asserts in transform.rs ==="
rg -n "abs\(\)\s*<\s*1e-9|1e-9|epsilon" crates/mohu-fft/src/transform.rs || true
# Search for required helper usage in repo
echo "=== usages of mohu-testing approx assert_allclose ==="
rg -n "mohu[-_]?testing::approx::assert_allclose|assert_allclose\(" crates | head -n 80
# Show cfg(test) location(s) in this file
echo "=== cfg(test) modules in transform.rs ==="
rg -n "cfg\(test\)" crates/mohu-fft/src/transform.rs || true
echo "=== file tail (last 80 lines) ==="
awk 'NR>$(wc -l < crates/mohu-fft/src/transform.rs)-80 {printf "%d\t%s\n", NR, $0}' crates/mohu-fft/src/transform.rsRepository: mohu-org/mohu
Length of output: 1143
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Broaden search for the approx helper used by the guidelines
echo "=== search mohu-testing approx helpers ==="
rg -n "approx::assert_allclose|assert_allclose|assert_all_close|allclose" crates | head -n 200 || true
echo "=== search for mohu_testing::approx module usage ==="
rg -n "mohu[-_]?testing::approx::" crates | head -n 200 || true
echo "=== list likely approx helper definitions (if present) ==="
rg -n "mod approx|pub mod approx|assert_.*allclose" crates/mohu-testing 2>/dev/null || trueRepository: mohu-org/mohu
Length of output: 1362
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== mohu-testing/src/lib.rs (top) ==="
sed -n '1,120p' crates/mohu-testing/src/lib.rs
echo "=== mohu-testing/src/approx.rs (or mod file) ==="
# Locate the approx module file
fd approx crates/mohu-testing/src -t f || true
# Show the approx module contents (limit to first 200 lines)
approx_file="$(fd approx crates/mohu-testing/src -t f | head -n 1 || true)"
if [ -n "${approx_file}" ]; then
echo "=== ${approx_file} (head) ==="
sed -n '1,240p' "${approx_file}"
fi
echo "=== search for assert_allclose definition inside mohu-testing ==="
rg -n "assert_allclose" crates/mohu-testing/src || trueRepository: mohu-org/mohu
Length of output: 2023
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== list approx module directory ==="
ls -ლა crates/mohu-testing/src/approx
echo "=== cat approx/mod.rs (top) ==="
if [ -f crates/mohu-testing/src/approx/mod.rs ]; then
sed -n '1,240p' crates/mohu-testing/src/approx/mod.rs
fi
echo "=== find definition of assert_allclose in mohu-testing ==="
rg -n "assert_allclose" crates/mohu-testing/src/approx crates/mohu-testing/src || true
echo "=== find re-export path used by macros (macro_rules / pub fn) ==="
rg -n "macro_rules!\s*assert_allclose|pub\s+fn\s+assert_allclose" crates/mohu-testing/src/approx crates/mohu-testing/src || trueRepository: mohu-org/mohu
Length of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== list approx module directory ==="
ls -la crates/mohu-testing/src/approx
echo "=== cat crates/mohu-testing/src/approx/mod.rs (top) ==="
if [ -f crates/mohu-testing/src/approx/mod.rs ]; then
sed -n '1,240p' crates/mohu-testing/src/approx/mod.rs
else
echo "No approx/mod.rs found"
fi
echo "=== search for assert_allclose definition ==="
rg -n "assert_allclose" crates/mohu-testing/src/approx crates/mohu-testing/src || true
echo "=== search for approx::assert_allclose usage patterns ==="
rg -n "assert_allclose!\(" crates | head -n 50Repository: mohu-org/mohu
Length of output: 190
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== find approx-related sources in crates/mohu-testing/src ==="
find crates/mohu-testing/src -maxdepth 3 -type f -name '*approx*' -print
echo "=== find approx module entry (approx.rs or approx/mod.rs) ==="
ls -la crates/mohu-testing/src | rg -n "approx" || true
ls -la crates/mohu-testing/src/approx* 2>/dev/null || true
find crates/mohu-testing/src -maxdepth 4 -type f \( -name 'mod.rs' -o -name '*.rs' \) -print | rg -n "approx" || true
echo "=== show pub mod approx from lib.rs (already known) ==="
rg -n "pub mod approx" crates/mohu-testing/src/lib.rs
echo "=== locate assert_allclose macro/function in mohu-testing ==="
rg -n "assert_allclose" crates/mohu-testing/src | head -n 200Repository: mohu-org/mohu
Length of output: 1183
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== show crates/mohu-testing/src/approx.rs ==="
wc -l crates/mohu-testing/src/approx.rs
sed -n '1,200p' crates/mohu-testing/src/approx.rs
echo "=== show crates/mohu-testing/src/approx (no extension) ==="
wc -l crates/mohu-testing/src/approx || true
sed -n '1,200p' crates/mohu-testing/src/approx || true
echo "=== search for assert_allclose macro definition in mohu-testing ==="
rg -n "assert_allclose!|macro_rules!\s*assert_allclose|pub\s+(use|macro|fn)\s+assert_allclose" crates/mohu-testing/src || trueRepository: mohu-org/mohu
Length of output: 766
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== list crates/mohu-testing/src/assert ==="
ls -la crates/mohu-testing/src/assert || true
find crates/mohu-testing/src/assert -maxdepth 3 -type f -print || true
echo "=== search for assert_allclose/allclose in mohu-testing ==="
rg -n "assert_allclose|allclose" crates/mohu-testing/src || true
echo "=== search for macro_rules for allclose/assert ==="
rg -n "macro_rules!\s*(assert|allclose)" crates/mohu-testing/src || true
echo "=== show crates/mohu-testing/src/assert.rs (if exists) ==="
if [ -f crates/mohu-testing/src/assert.rs ]; then
wc -l crates/mohu-testing/src/assert.rs
sed -n '1,220p' crates/mohu-testing/src/assert.rs
fi
echo "=== show crates/mohu-testing/src/assert/mod.rs (if exists) ==="
if [ -f crates/mohu-testing/src/assert/mod.rs ]; then
wc -l crates/mohu-testing/src/assert/mod.rs
sed -n '1,260p' crates/mohu-testing/src/assert/mod.rs
fiRepository: mohu-org/mohu
Length of output: 950
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== ls mohu-testing/src ==="
ls -la crates/mohu-testing/src
echo "=== find mohu-testing/src (type f) maxdepth 3 ==="
find crates/mohu-testing/src -maxdepth 3 -type f -print | sort
echo "=== inspect mohu-testing/src/lib.rs (rest) ==="
sed -n '120,260p' crates/mohu-testing/src/lib.rs
echo "=== search for assert_allclose macro/function definitions ==="
rg -n "assert_allclose!" crates/mohu-testing/src || true
rg -n "macro_rules!\s*assert_allclose" crates/mohu-testing/src || true
rg -n "assert_allclose\s*[\(;]" crates/mohu-testing/src || true
echo "=== search repo for assert_allclose! usage ==="
rg -n "assert_allclose!\s*\(" crates || true
rg -n "mohu_testing::assert_allclose!\s*\(" crates || true
rg -n "mohu[-_]?testing::approx::assert_allclose" crates || trueRepository: mohu-org/mohu
Length of output: 1790
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== mohu-testing/src/lib.rs (full for macro exports area) ==="
sed -n '1,240p' crates/mohu-testing/src/lib.rs
echo "=== mohu-testing/src/lib.rs (later section) ==="
sed -n '240,520p' crates/mohu-testing/src/lib.rs || true
echo "=== dump any pub use / macro exports referencing assert_allclose ==="
rg -n "pub\s+use\s+.*assert_allclose|assert_allclose" crates/mohu-testing/src/lib.rs || trueRepository: mohu-org/mohu
Length of output: 2137
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== repo-wide search for assert_allclose! ==="
rg -n "assert_allclose!" . || true
echo "=== repo-wide search for assert_allclose (without ! ) ==="
rg -n "assert_allclose" . || true
echo "=== repo-wide search for approx::assert_allclose ==="
rg -n "approx::assert_allclose" . || true
echo "=== repo-wide search for Buffer::allclose usage (likely alternative helper) ==="
rg -n "Buffer::allclose|\.allclose\(" crates | head -n 100 || true
echo "=== inspect mohu-buffer allclose implementation (signature) ==="
sed -n '1520,1605p' crates/mohu-buffer/src/buffer.rsRepository: mohu-org/mohu
Length of output: 5262
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== CONTRIBUTING.md around mohu_testing::approx::assert_allclose ==="
rg -n "mohu_testing::approx::assert_allclose" CONTRIBUTING.md
line=$(rg -n "mohu_testing::approx::assert_allclose" CONTRIBUTING.md | head -n 1 | cut -d: -f1)
start=$((line-20))
end=$((line+60))
awk -v s="$start" -v e="$end" 'NR>=s && NR<=e {print NR ":" $0}' CONTRIBUTING.md
echo "=== AGENTS.md around float comparison guideline ==="
rg -n "Float comparisons in tests must use mohu_testing::approx::assert_allclose" AGENTS.md
line=$(rg -n "Float comparisons in tests must use mohu_testing::approx::assert_allclose" AGENTS.md | head -n 1 | cut -d: -f1)
start=$((line-20))
end=$((line+60))
awk -v s="$start" -v e="$end" 'NR>=s && NR<=e {print NR ":" $0}' AGENTS.md
echo "=== CLAUDE.md around float comparison guideline ==="
rg -n "mohu-testing::approx::assert_allclose" CLAUDE.md
line=$(rg -n "mohu-testing::approx::assert_allclose" CLAUDE.md | head -n 1 | cut -d: -f1)
start=$((line-20))
end=$((line+60))
awk -v s="$start" -v e="$end" 'NR>=s && NR<=e {print NR ":" $0}' CLAUDE.mdRepository: mohu-org/mohu
Length of output: 2411
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== inspect crates/mohu-testing/src/approx.rs ==="
wc -l crates/mohu-testing/src/approx.rs
sed -n '1,120p' crates/mohu-testing/src/approx.rs
echo "=== inspect crates/mohu-testing/src/approx (no extension) ==="
wc -c crates/mohu-testing/src/approx
sed -n '1,120p' crates/mohu-testing/src/approx || true
echo "=== search assert_allclose in crates/mohu-testing/src/approx* ==="
rg -n "assert_allclose" crates/mohu-testing/src/approx* crates/mohu-testing/src || true
echo "=== inspect CONTRIBUTING/AGENTS/CLAUDE exact lines about assert_allclose ==="
for f in AGENTS.md CLAUDE.md CONTRIBUTING.md; do
echo "---- $f (matches) ----"
rg -n "mohu_testing::approx::assert_allclose|mohu-testing::approx::assert_allclose|assert_allclose" "$f" || true
done
echo "=== inspect CRATE_MAP entry about assert_allclose ==="
rg -n "assert_allclose" CRATE_MAP.md | head -n 50Repository: mohu-org/mohu
Length of output: 320
Replace manual float epsilon assertions with mohu_testing::approx::assert_allclose
In crates/mohu-fft/src/transform.rs (tests::roundtrip_fft_ifft, lines 72-75), replace the assert!((a.re - b.re).abs() < 1e-9) / assert!((a.im - b.im).abs() < 1e-9) checks with mohu_testing::approx::assert_allclose to match the project’s required float-comparison helper.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/mohu-fft/src/transform.rs` around lines 72 - 75, In
tests::roundtrip_fft_ifft replace the two manual epsilon asserts inside the for
(a, b) in input.iter().zip(back.iter()) loop with a single call to
moh_testing::approx::assert_allclose so the project float-comparison helper is
used; compare the complex values from input and back (a and b) with tolerance
1e-9 (or the helper’s equivalent args) by invoking assert_allclose on the
pair/values instead of using (a.re - b.re).abs() and (a.im - b.im).abs() checks.
Signed-off-by: sonusharma6-dsa <ramprashadkumar11@gmail.com>
Signed-off-by: sonusharma6-dsa <ramprashadkumar11@gmail.com>
Author
|
Pushed formatting fixes to address Format failures; new CI runs have been triggered by this commit. I will follow up on any remaining Clippy, Unused Deps, or Cargo Deny failures. |
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.
Implements 1D FFT/IFFT and frequency helpers in mohu-fft. This branch is a fresh replacement for PR #232 and aims to address CI and review feedback.\n\nCloses #225.
Summary by CodeRabbit
New Features
Refactor
Style
Chores
Tests