Skip to content

Fix: rework nondet math operations#796

Merged
amimart merged 21 commits intodevfrom
arnaud/engn-3613-rework-nondet-math
May 7, 2025
Merged

Fix: rework nondet math operations#796
amimart merged 21 commits intodevfrom
arnaud/engn-3613-rework-nondet-math

Conversation

@amimart
Copy link
Contributor

@amimart amimart commented Apr 8, 2025

Purpose of Changes and their Description

Re-implement in a deterministic way some math func, here are the details on the chosen algorithms:

  • Exp: Uses a Taylor polynomial with a lookup table for powers of 10^(1/n) of size 300 which seems sufficient for 34 precision digits;
  • Ln: Implements the Newton's method that uses exp 3 times;
  • Log10: Use d = ln(x) * 1/ln(10);
  • Pow: Implement apd's algorithm but using our Ln and Exp impls;

The Exp10 func has been removed as not used.

The algorithms choices and implementations were made with the idea on improving performances, we are now much faster, see the benchmarks below. In terms of accuracy it is almost identical to previous implementations.

The perf improvements helped the network inferences calculation to drop from 250ms to 100ms.

Benchmarks before:

BenchmarkLn/input_1.2-12                    12574	     95334 ns/op
BenchmarkLog10/input_1.2-12              17979	     66241 ns/op
BenchmarkExp/input_1.2-12                  44658	     26917 ns/op
BenchmarkPow/input_1.2_1.2-12          10000	    103102 ns/op

Benchmarks after:

BenchmarkLn/input_1.2-12                    45258	     26759 ns/op
BenchmarkLog10/input_1.2-12              44979	     27504 ns/op
BenchmarkExp/input_1.2-12                  153902	      7737 ns/op
BenchmarkPow/input_1.2_1.2-12          36795	     32447 ns/op

Are these changes tested and documented?

  • If tested, please describe how. If not, why tests are not needed.
  • If documented, please describe where. If not, describe why docs are not needed.
  • Added to Unreleased section of CHANGELOG.md?

@amimart amimart force-pushed the arnaud/engn-3613-rework-nondet-math branch 2 times, most recently from fa50d63 to 55bdd0a Compare April 8, 2025 20:49
@amimart amimart force-pushed the arnaud/engn-3613-rework-nondet-math branch 2 times, most recently from c80eead to 986936e Compare April 22, 2025 17:37
@amimart amimart marked this pull request as ready for review April 22, 2025 17:54
@amimart amimart force-pushed the arnaud/engn-3613-rework-nondet-math branch 2 times, most recently from e1d1e7f to b495331 Compare May 1, 2025 06:37
Copy link
Contributor

@xmariachi xmariachi left a comment

Choose a reason for hiding this comment

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

Left a comment on testing. lgtm in general, good work!

Copy link
Contributor

@guilherme-brandao guilherme-brandao left a comment

Choose a reason for hiding this comment

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

Minor comment. Already approving! Nice work!

@amimart amimart force-pushed the arnaud/engn-3613-rework-nondet-math branch from b495331 to a50278c Compare May 6, 2025 17:24
@amimart amimart merged commit 544a3c1 into dev May 7, 2025
11 checks passed
@amimart amimart deleted the arnaud/engn-3613-rework-nondet-math branch May 7, 2025 13:35
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