Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf wExp #12

Merged
merged 16 commits into from
Sep 4, 2023
Merged

perf wExp #12

merged 16 commits into from
Sep 4, 2023

Conversation

MathisGD
Copy link
Contributor

@MathisGD MathisGD commented Sep 3, 2023

Fixes #5. second alternative for an optimized wExp (range reduction + low order Taylor), much simpler.

image

the reason why it is more efficient than the other one is probably because the Taylor order is low, meaning that it is not very precise but I think that it is enough for what we do.

@MathisGD MathisGD self-assigned this Sep 3, 2023
@MathisGD MathisGD changed the title perf wExp (2) perf wExp (2) Sep 3, 2023
@MathisGD MathisGD requested review from a team, Rubilmax, MerlinEgalite, Jean-Grimal, QGarchery and peyha and removed request for a team September 3, 2023 23:58
Copy link
Contributor

@MerlinEgalite MerlinEgalite left a comment

Choose a reason for hiding this comment

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

Way easier to understand and reason about. I'm in favor of this version.

src/irm/libraries/MathLib.sol Outdated Show resolved Hide resolved
src/irm/libraries/MathLib.sol Outdated Show resolved Hide resolved
test/irm/MathLibTest.sol Outdated Show resolved Hide resolved
src/irm/libraries/MathLib.sol Show resolved Hide resolved
src/irm/libraries/MathLib.sol Outdated Show resolved Hide resolved
src/irm/libraries/MathLib.sol Outdated Show resolved Hide resolved
src/irm/libraries/MathLib.sol Outdated Show resolved Hide resolved
src/irm/libraries/MathLib.sol Outdated Show resolved Hide resolved
Copy link
Contributor

@Jean-Grimal Jean-Grimal left a comment

Choose a reason for hiding this comment

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

I'm in favor of this implementation. It's way more readable than #11 and I also think that this method is very smart.

@MathisGD
Copy link
Contributor Author

MathisGD commented Sep 4, 2023

Ok so I was doing 2 mistakes:

  • there is no pb when x < -ln(2^256)
  • when x > ln(2^256) the function does not revert but returns zero instead

I fixed and tested both

test/irm/MathLibTest.sol Outdated Show resolved Hide resolved
test/irm/MathLibTest.sol Outdated Show resolved Hide resolved
@MathisGD
Copy link
Contributor Author

MathisGD commented Sep 4, 2023

New snapshot (but the test have changed)

image

This was referenced Sep 4, 2023
@MathisGD MathisGD changed the title perf wExp (2) perf wExp Sep 4, 2023
src/irm/libraries/MathLib.sol Outdated Show resolved Hide resolved
src/irm/libraries/MathLib.sol Show resolved Hide resolved
test/irm/MathLibTest.sol Show resolved Hide resolved
test/irm/MathLibTest.sol Show resolved Hide resolved
@MathisGD MathisGD merged commit 91b15e9 into main Sep 4, 2023
2 checks passed
@MathisGD MathisGD deleted the perf/wExp-2 branch September 4, 2023 16:46
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.

Optimize wExp
4 participants