Skip to content

Math: Complex: Calculate complex phase angle with sofm_atan2()#10629

Open
singalsu wants to merge 5 commits intothesofproject:mainfrom
singalsu:math_add_atan2
Open

Math: Complex: Calculate complex phase angle with sofm_atan2()#10629
singalsu wants to merge 5 commits intothesofproject:mainfrom
singalsu:math_add_atan2

Conversation

@singalsu
Copy link
Collaborator

No description provided.

@singalsu singalsu marked this pull request as ready for review March 18, 2026 10:25
Copilot AI review requested due to automatic review settings March 18, 2026 10:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a fixed-point sofm_atan2_32b() implementation (Q1.31 inputs → Q3.29 radians) and switches complex-to-polar phase calculation to use it, along with new unit-test vectors and build integration.

Changes:

  • Add src/math/atan2.c implementing sofm_atan2_32b() via a Remez polynomial approximation.
  • Update sofm_icomplex32_to_polar() to compute phase using sofm_atan2_32b() instead of acos_fixed_32b().
  • Add and wire up atan2 unit tests + generated tables; add Q_CONVERT_QTOD() for double-based comparisons in tests.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/ztest/unit/math/basic/trigonometry/trig_test.c Adds sofm_atan2_32b() unit test using generated vectors.
test/ztest/unit/math/basic/trigonometry/atan2_tables.m Octave/MATLAB script to generate fixed-point test vectors/header.
test/ztest/unit/math/basic/trigonometry/atan2_tables.h Generated test vectors for atan2 validation.
test/ztest/unit/math/basic/trigonometry/CMakeLists.txt Links src/math/atan2.c into trig unit-test build.
test/ztest/unit/math/basic/complex/test_complex_polar.c Tightens angle tolerance to match improved phase calculation.
test/ztest/unit/math/basic/complex/CMakeLists.txt Links src/math/atan2.c into complex unit-test build.
src/math/complex.c Switches polar angle computation to sofm_atan2_32b().
src/math/atan2.c New implementation of fixed-point 4-quadrant atan2.
src/math/Kconfig Adds MATH_ATAN2 config and ensures MATH_COMPLEX selects it.
src/math/CMakeLists.txt Conditionally builds atan2.c under CONFIG_MATH_ATAN2.
src/include/sof/math/trig.h Declares sofm_atan2_32b() and documents it (needs consistency fixes).
src/include/sof/audio/format.h Adds Q_CONVERT_QTOD() macro used by tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a fixed-point four-quadrant atan2 implementation and switches complex polar conversion to use it (instead of acos-based reconstruction), with accompanying unit tests and build/config wiring.

Changes:

  • Introduce sofm_atan2_32b() (Q1.31 inputs → Q3.29 radians output) behind CONFIG_MATH_ATAN2.
  • Update sofm_icomplex32_to_polar() to compute phase via sofm_atan2_32b(imag, real).
  • Add Ztest coverage for sofm_atan2_32b() plus generated test vectors/tables.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/ztest/unit/math/basic/trigonometry/trig_test.c Adds sofm_atan2_32b() unit test using generated vectors.
test/ztest/unit/math/basic/trigonometry/atan2_tables.m Adds Octave/MATLAB script to generate atan2 test vectors header.
test/ztest/unit/math/basic/trigonometry/atan2_tables.h Adds generated Q31 input vectors and Q29 reference angles.
test/ztest/unit/math/basic/trigonometry/CMakeLists.txt Links atan2.c into trigonometry unit test build.
test/ztest/unit/math/basic/complex/test_complex_polar.c Tightens angle tolerance and adjusts test info prints.
test/ztest/unit/math/basic/complex/CMakeLists.txt Links atan2.c into complex unit test build.
src/math/complex.c Switches polar angle computation to sofm_atan2_32b().
src/math/atan2.c New polynomial-based atan2 implementation + exported symbol.
src/math/Kconfig Adds MATH_ATAN2 option; wires complex math to select needed deps.
src/math/CMakeLists.txt Builds atan2.c when CONFIG_MATH_ATAN2 is enabled.
src/include/sof/math/trig.h Declares sofm_atan2_32b() and documents it.
src/include/sof/audio/format.h Adds Q_CONVERT_QTOD() helper macro for tests/diagnostics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +241 to +245
double delta_max = 0.0;
int32_t result_q29_max = 0;
int32_t result_q29;
int i_max = -1;
int i;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep

Comment on lines +257 to +259
zassert_true(delta <= CMP_TOLERANCE_ATAN2,
"sofm_atan2 failed for input %d: (%d, %d) result %d",
i, atan2_test_y[i], atan2_test_x[i], result_q29);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Comment on lines +393 to +394
* atan(z) = z * (C0 + z^2 * (C1 + z^2 * (C2 + z^2 * C3)))
* with Remez minimax coefficients on [0, 1].
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that was still missing from polynomial order increase for sufficient precision.

Comment on lines +1 to +3
# SPDX-License-Identifier: BSD-3-Clause
#
# Copyright(c) 2026 Intel Corporation.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, good to change. Matlab would error with # - comment, while Octave accepts it.

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you resolve the copilot opens. Thanks !

This patch adds the four quadrant arctan function version.
It is useful for fast computation of phase angle -pi to +pi
in radians from a complex number (real, imaginary) value. The
approximation uses a 9th order polynomial that is implemented
with Horner's rule evaluation.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The usage of Q_CONVERT_QTOF() causes build issue with llvm
toolchain when assigned to a double type in ztest code:

error: implicit conversion increases floating-point precision:
'float' to 'double' [-Werror,-Wdouble-promotion]

Since a double is needed to fully match the accuracy of 32 bit
fraction, a new macro Q_CONVERT_QTOD() is added for the purpose.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a fixed-point four-quadrant atan2 implementation and switches complex phase-angle calculation to use it, with accompanying unit tests and build/config wiring.

Changes:

  • Introduces sofm_atan2_32b() (Q1.31 inputs → Q3.29 radians output) implemented via a degree-9 minimax polynomial.
  • Updates complex-to-polar conversion to compute angle via sofm_atan2_32b() instead of acos-based logic.
  • Adds table-driven unit tests and supporting generated reference tables; wires atan2.c into relevant test/build targets and Kconfig/CMake.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/ztest/unit/math/basic/trigonometry/trig_test.c Adds sofm_atan2_32b() unit test using generated vectors.
test/ztest/unit/math/basic/trigonometry/atan2_tables.m MATLAB/Octave script to generate atan2 test vectors and references.
test/ztest/unit/math/basic/trigonometry/atan2_tables.h Generated test vectors/reference angles for atan2 test.
test/ztest/unit/math/basic/trigonometry/CMakeLists.txt Links src/math/atan2.c into trigonometry unit test app.
test/ztest/unit/math/basic/complex/test_complex_polar.c Tightens angle tolerance and adjusts info prints.
test/ztest/unit/math/basic/complex/CMakeLists.txt Links src/math/atan2.c into complex unit test app.
src/math/complex.c Uses sofm_atan2_32b() for phase angle in sofm_icomplex32_to_polar().
src/math/atan2.c New implementation of sofm_atan2_32b() polynomial atan2 with quadrant mapping.
src/math/Kconfig Adds MATH_ATAN2 option; ensures MATH_COMPLEX selects required deps.
src/math/CMakeLists.txt Builds atan2.c when CONFIG_MATH_ATAN2 is enabled.
src/include/sof/math/trig.h Declares sofm_atan2_32b() and documents polynomial form/error.
src/include/sof/audio/format.h Adds Q_CONVERT_QTOD() macro for Q-format to double conversion.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

i_max = i;
}
zassert_true(delta <= CMP_TOLERANCE_ATAN2,
"sofm_atan2 failed for input %d: (%d, %d) result %d, expected %d",
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion failure message says "sofm_atan2 failed" but the function under test is sofm_atan2_32b(). Updating the message to the correct function name will make failures easier to interpret (especially when multiple atan/atan2 implementations exist).

Suggested change
"sofm_atan2 failed for input %d: (%d, %d) result %d, expected %d",
"sofm_atan2_32b failed for input %d: (%d, %d) result %d, expected %d",

Copilot uses AI. Check for mistakes.
Comment on lines +247 to +251
for (i = 0; i < ATAN2_TEST_TABLE_SIZE; ++i) {
result_q29 = sofm_atan2_32b(atan2_test_y[i], atan2_test_x[i]);
result = Q_CONVERT_QTOD(result_q29, 29);
reference = Q_CONVERT_QTOD(atan2_test_ref[i], 29);
delta = fabs(reference - result);
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test_atan2 coverage currently relies entirely on the generated random table. That table does not include key edge cases like the origin (0,0) and axis-aligned inputs where x == 0 or y == 0, so the function's explicit origin handling and quadrant/axis boundary mapping aren’t exercised. Consider adding a few explicit zasserts for these cases (and expected ±pi/2, 0, ±pi) alongside the table-driven loop.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, done in tables data generate script with some forced x or y values in begin.

This patch adds the test for the function. The input values
table for atan are Q1.31 (x, y) pair numbers and reference
angle values in Q3.29 radians. The table is computed with
the double precision atan2() function in Octave and converted
to fractional integers. See script atan2_tables.m.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch updates the phase angle calculation in function
sofm_icomple32_to_polar() with more efficient sofm_atan2()
function. Calculate of angle with arctan function avoids the
64 bit divide in the previous arccos function based calculation.

In 48 kHz stereo STFT processing with 1024 size FFT and hop of
256 this change saves about 14 MCPS in MTL platform (204 MCPS
to 190 MCPS).

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The atan2() function in sofm_ipolar32_to_complex() helped to
increase accuracy, so the tolerance for error could be decreased
to about 0.001 degrees (2.0e-5 radians).

Also the print format for achieved accuracy is improved for nicer
appearance.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
@singalsu singalsu requested a review from Copilot March 20, 2026 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants