zig icon indicating copy to clipboard operation
zig copied to clipboard

rewrite math.complex

Open expikr opened this issue 1 year ago • 3 comments

Closes https://github.com/ziglang/zig/issues/19207

This decouples the math.complex namespace away from a centralized type definition, instead providing a bring-your-own-type collection of utility functions with no dependency on the specific types' declarations.

The math.Complex type constructor is removed, instead encouraging programmers to pick-and-choose the subset of provided functions to incorporate into their type of choice.

This remains the case even once https://github.com/ziglang/zig/issues/16278 is implemented, as the collection already makes no assumptions about the concrete type beyond the presence of a re and im field.

The source files are reorganized such that public-facing dispatch interfaces are all inside complex.zig, the subdirectory instead only containing datatype-specific internal implementations that is only @imported inside the dispatch function body.

The datatype-specific internal implementations are entirely oblivious to any sort of type constructs, receiving inputs as multiple scalar arguments and returning tuples of values.

Future Incremental Improvements:

  • [x] audit the original musl implementation, reimplement from first-principles where appropriate
    • [x] ldexp_cexp (no error)
    • [x] exp (no error)
    • [x] cosh (3 errors)
    • [x] sinh (no error)
    • [x] tanh (no error)
    • [x] sqrt (2 errors)
    • [x] atan (2 errors)
  • [x] fill out fallback branches with naive algebraic expressions
    • [x] atan
    • [x] tanh
    • [x] sqrt
  • [ ] create exhaustive numerical precision tests for all datatype-specific impls

expikr avatar Jun 19 '24 06:06 expikr

c.c. @tiehuis since this largely overhauls https://github.com/ziglang/zig/pull/949 and also partly progresses #19476

expikr avatar Jul 20 '24 20:07 expikr

Happy to merge the fixes separately as these look good, but will need a bit more time to check over the api changes.

I generally think they look promising, but given the proposal isn't accepted and it changes quite a bit want a closer look.

tiehuis avatar Jul 22 '24 07:07 tiehuis

My primary question is what does removing the Complex type actually give us?

A user having to create a structurally matching type seems odd. I can't think of any great use-cases that this approach gives over the dedicated type in 99% of cases. We also lose documentation on a base type we could refer to.

tiehuis avatar Jul 27 '24 11:07 tiehuis

Author has not complied with my request to limit to 1 open PR at a time, is now in timeout for 1 year.

andrewrk avatar Jul 29 '24 03:07 andrewrk