Skip to content

feat(math) Add complex number support for the @std/math library#7122

Open
Definitely-Not-A-Dolphin wants to merge 17 commits into
denoland:mainfrom
Definitely-Not-A-Dolphin:main
Open

feat(math) Add complex number support for the @std/math library#7122
Definitely-Not-A-Dolphin wants to merge 17 commits into
denoland:mainfrom
Definitely-Not-A-Dolphin:main

Conversation

@Definitely-Not-A-Dolphin

Copy link
Copy Markdown

No description provided.

@CLAassistant

CLAassistant commented May 3, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions Bot added the math label May 3, 2026
@codecov

codecov Bot commented May 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (cd03740) to head (ccabb07).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #7122       +/-   ##
==========================================
- Coverage   94.61%       0   -94.62%     
==========================================
  Files         634       0      -634     
  Lines       51799       0    -51799     
  Branches     9329       0     -9329     
==========================================
- Hits        49009       0    -49009     
+ Misses       2216       0     -2216     
+ Partials      574       0      -574     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Definitely-Not-A-Dolphin Definitely-Not-A-Dolphin changed the title Add complex number support for the @std/math library feat(complex) Add complex number support for the @std/math library May 3, 2026
@Definitely-Not-A-Dolphin

Definitely-Not-A-Dolphin commented May 3, 2026

Copy link
Copy Markdown
Author

Excuse me for my bad commit messages earlier, I forgot that this was a professional setting. A few topics of discussion here:

Structure

The entire module contains a Complex class that is used to create complex numbers. The functions for complex numbers, (arithmatic, absolute value, argument, trig) are static methods on that class. I made that decision for two reasons:

  1. The "real" number functions are collected in the Math namespace, so I thought it made sense to collect all the complex number functions in a Complex object.
  2. When importing the class, you don't have to import 30 different functions seperately, which looks bad and is terrible for name collisions.

Documentation

The deno contributing guidelines state that when applicable, all exported symbols must be documented according to the following rules:

  1. A short description, then any further details in new paragraph(s).
  2. A @typeParam tag for each type parameter.
  3. A @param tag for each parameter.
  4. A @returns tag for the return value.
  5. At least one example code snippet using the
    @example tag and a title. For simple
    examples which don't need a description, "Usage" is an acceptable title. See
    Example code snippets below for further guidance.

The first rule I understand, but the rules about the JSDoc documentation feel a bit unnecessary to me. Unlike all the other libraries, which are about programming / computer topics, this is math. Function names about computer topics are always at least a little vague, but math functions aren't. Sine is always sine, without any ambiguity.

Also, is it really necessary to include example cases for all 24 trig functions? Not only are there a lot of them, they are all pretty abstract. I don't think that adding examples would be of any additional value.

Trig function names

Should inverse trig functions be called asin, acos, atan, or arcsin, arccos, arctan? My math side says the latter, but my computer side says the former.

@Definitely-Not-A-Dolphin

Copy link
Copy Markdown
Author

This PR is also a draft because

  1. I still need to write test cases, and
  2. The above mentioned discussions still need to be done

@bartlomieju bartlomieju left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the contribution — complex number support is a genuinely useful addition. Marking this as a high-level review while it's still a draft. A few overarching points before the inline notes:

API placement. @std/math is stable, so new APIs of this scope need to be introduced under an unstable-complex export first (see the precedent in data-structures/unstable, async/unstable-*, etc.). The current diff also doesn't wire anything into math/deno.json, so the new API isn't actually exposed via the package entrypoints.

API shape. Using Complex as a namespace of ~35 static methods isn't idiomatic for std — the package prefers one named function per file (see clamp.ts, modulo.ts, round_to.ts), or, for value types, instance methods (a.add(b).mul(c)). Could you open an issue to settle the shape before we keep iterating? The choice between static-only / instance methods / per-file functions has big downstream consequences.

Scope. The author flags ~12 inverse functions as broken (branch cuts). I'd strongly suggest dropping those from the first PR and adding them in a follow-up once correct — shipping known-broken functions in an initial release sets a bad precedent.

Docs/tests. Every public API in std requires an @example Usage block with executable code that uses assertEquals / assertAlmostEquals (see math/clamp.ts:12-19). None of the new methods have examples, and the test file only covers constructor and add (with incorrect expectations — see inline). Before this is merge-ready every operation needs round-tripping tests (conj(conj(z)) === z, mul(i, i) === -1, exp(i*PI) ≈ -1, etc.).

Detailed correctness issues inline.

Comment thread math/complex.ts Outdated

/**
* Returns the sum of the supplied complex numbers.
*

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Bug: the imaginary component uses num.real instead of num.imag. The existing test happens to pass because the imaginary parts in the inputs equal the real parts of the next operand. Should be:

sum = new Complex(sum.real + num.real, sum.imag + num.imag);

Comment thread math/complex.ts Outdated
this.#imag = imag ?? 0;
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two problems here:

  1. Typo — Number.isNaN(z.real) || Number.isNaN(z.real) checks real twice; the second should be z.imag.
  2. This method is never called anywhere.

More importantly, the singleton-based NaN propagation throughout the file is broken: Complex.#complexNaN is one specific instance, but new Complex(NaN, ...) returns a new instance with NaN fields. Reference comparisons like nums.includes(this.#complexNaN) and x === this.#complexNaN in add/sub/mul/div will never match a user-constructed NaN. Replace the identity checks with Number.isNaN(z.real) || Number.isNaN(z.imag).

Comment thread math/complex.ts Outdated
this.#imag = NaN;
} else {
this.#real = real;
this.#imag = imag ?? 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See note on #NaNChecker — this singleton can't actually be detected via === against values produced by new Complex(NaN), so all the nums.includes(this.#complexNaN) / x === this.#complexNaN guards below are no-ops in practice.

Comment thread math/complex.ts Outdated
}

/**
* Returns the square of the absolute value of a complex number.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

JSDoc copy-paste: this is arg, not absSquared. The @returns line still says "the square of the absolute value of the supplied number."

Comment thread math/complex.ts Outdated
}

get imag() {
return this.#imag;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Constructor only special-cases NaN, not Infinity. The tests on complex_test.ts:13-22 assume new Complex(Infinity, 1) collapses to (Infinity, Infinity), which doesn't happen. Either:

  • decide that Complex preserves component values verbatim (recommended — matches how IEEE complex usually behaves), and fix the tests, or
  • add explicit Infinity normalization here and document it.

Also: imag is number | undefined, and Number.isNaN(undefined) is false, so the current branch is fine — but it's worth narrowing imag to number by defaulting earlier.

Comment thread math/complex.ts Outdated

return new Complex(expReal * Math.cos(z.imag), expReal * Math.sin(z.imag));
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggest dropping asin/acos/atan/acot/asec/acsc and their hyperbolic counterparts from this PR entirely — you've flagged them as broken at the top of the file. Land the well-tested operations first and add inverse trig in a follow-up once branch cuts are correct.

await t.step("constructor", () => {
assertEquals(new Complex(5, 1).imag, 1);
assertEquals(new Complex(5, 1).real, 5);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These Infinity assertions will fail as written — the constructor doesn't collapse (Infinity, 1) to (Infinity, Infinity), it stores the components as given. Either change the constructor's contract or update these expectations to new Complex(Infinity, 1) etc.

Comment thread math/complex_test.ts Outdated

assertEquals(
Complex.add(new Complex(2, 3), new Complex(2, Infinity)),
new Complex(0),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This expectation is mathematically wrong: Complex(2, 3) + Complex(2, Infinity) = (4, Infinity), not (0, 0). (It currently "passes" only because of the sum.imag + num.real bug in add() combined with NaN comparison quirks.)

import { Complex } from "./complex.ts";
import { assertEquals } from "@std/assert";

Deno.test("Complex", async (t) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Test coverage needs to grow significantly before this is merge-ready — only constructor and add are exercised. At minimum every method needs a test, ideally using algebraic identities (conj(conj(z)) === z, mul(i, i) === Complex(-1), exp(Complex(0, PI)) ≈ Complex(-1), sqrt(z) * sqrt(z) === z, etc.) with assertAlmostEquals from @std/assert for floating-point comparisons.

Comment thread math/mod.ts Outdated
* @module
*/

export * from "./complex.ts";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two issues:

  1. @std/math is a stable package; new APIs of this scope must land under an unstable-complex subpath first (e.g. ./unstable-complex.ts exported as "./unstable-complex").
  2. The new file isn't added to math/deno.json's exports map, so even users who want it can't import it as @std/math/complex.

@Definitely-Not-A-Dolphin Definitely-Not-A-Dolphin marked this pull request as ready for review June 26, 2026 16:06
@Definitely-Not-A-Dolphin

Copy link
Copy Markdown
Author

I finished all the functions and made all the tests pass after not having a lot of time to work on it due to school, but here I am. Here are some choices I made.

Static methods

It seemed logical for me to put all the static math methods on the class is because the of Math namespace so I thought it was fitting. The a.add(b) style didn't occur to me, but that does look prettier, but only for binary operations. I know that in some languages like Rust but in my opinion that looks really cursed. For the rest I am a fan of instance methods for those, and static methods for unary operators.

Inverse functions

At the time of reviewing the pr was unfinished, but now I managed to make all inverse functions work, except for the cotangent, secant and cosecant, including their hyperbolic forms. I did this because I couldn't find a clear consensus as to where the branch cut was. For example, my two most used tools for this, desmos and wolframalpha, disargreed on some hyperbolic arccotangent values, which is why I omitted it plus others.

@Definitely-Not-A-Dolphin

Copy link
Copy Markdown
Author

Maybe doing the a.fun() syntax over Complex.fun(a) is better also for unary operators for it is a little more terse

@Definitely-Not-A-Dolphin Definitely-Not-A-Dolphin changed the title feat(complex) Add complex number support for the @std/math library feat(math) Add complex number support for the @std/math library Jun 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants