Skip to content

Conversation

@engn33r
Copy link
Contributor

@engn33r engn33r commented Dec 14, 2023

✅ PR Checklist

  • Because this PR includes a bug fix, relevant tests have been included.
  • Because this PR includes a new feature, the change was previously discussed in an issue or in the discussions section.
  • I didn't do anything of this.

This PR is for a very small optimization that is possible by increasing the range of values that will take the short logic path of wad_exp() and return zero. I am not sure how the previously chosen value of -42_139_678_854_452_767_551 was selected, but these tests demonstrate the actual turning point when the result of the current wad_exp() implementation transitions from 1 to zero.

    function testWadExpZeroPoint() public {
        assertEq(math.wad_exp(-41_446_531_673_892_822_312), 1);
        assertEq(math.wad_exp(-41_446_531_673_892_822_313), 0);
    }

🐶 Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@pcaversaccio pcaversaccio added this to the 0.0.5 milestone Dec 14, 2023
@pcaversaccio pcaversaccio added the optimisation ⚡️ Code optimisations (e.g. gas improvements) label Dec 14, 2023
@pcaversaccio pcaversaccio self-requested a review December 14, 2023 13:50
@pcaversaccio pcaversaccio self-assigned this Dec 14, 2023
@pcaversaccio pcaversaccio changed the title Change value for short path to return zero ⚡️ Change Value for Short Path to Return Zero in wad_exp Dec 14, 2023
@pcaversaccio
Copy link
Owner

pcaversaccio commented Dec 14, 2023

@engn33r great catch! The original number stems from Remco's article: https://xn--2-umb.com/22/exp-ln/index.html.

Can you please adjust the tests accordingly:

-  assertEq(math.wad_exp(-42_139_678_854_452_767_551), 0);
+ assertEq(math.wad_exp(-41_446_531_673_892_822_313), 0);
+ assertEq(math.wad_exp(-41_446_531_673_892_822_312), 1);

and also update the .gas-snapshot file (run forge snapshot). After that I will quickly make a CHANGELOG entry and merge.

@engn33r
Copy link
Contributor Author

engn33r commented Dec 14, 2023

I made the requested changes.

Thanks for sharing the source, I didn't realize that's where the number originated from. I see solmate and solady are using the same value, I'll verify the same change can happen in their implementations and if so I'll link to this PR.

@pcaversaccio
Copy link
Owner

I made the requested changes.

Thanks for sharing the source, I didn't realize that's where the number originated from. I see solmate and solady are using the same value, I'll verify the same change can happen in their implementations and if so I'll link to this PR.

FWIW, I already shared it with @Vectorized!

@Vectorized
Copy link

@engn33r Beautiful optimization!

@engn33r
Copy link
Contributor Author

engn33r commented Dec 14, 2023

Solmate PR and solady PR created

@pcaversaccio
Copy link
Owner

Solmate PR created and solady PR inbound (2 mins)

Sweet, will merge tomorrow (busy day due to Ledger...)

Signed-off-by: sudo rm -rf --no-preserve-root / <pcaversaccio@users.noreply.github.com>
@pcaversaccio pcaversaccio changed the title ⚡️ Change Value for Short Path to Return Zero in wad_exp ⚡️ Optimise Zero Point in wad_exp Dec 14, 2023
@pcaversaccio
Copy link
Owner

For anyone who wants to understand how to replicate this using Titanoboa, here a step-by-step guide:

~$ pip install git+https://github.com/vyperlang/titanoboa
~$ git clone https://github.com/pcaversaccio/snekmate.git
~$ cd snekmate/src/utils/
~$ python
>>> import boa
>>> c = boa.load("Math.vy")
>>> c.wad_exp(-41_446_531_673_892_822_312)
1
>>> c.wad_exp(-41_446_531_673_892_822_313)
0

@pcaversaccio pcaversaccio changed the title ⚡️ Optimise Zero Point in wad_exp ⚡️ Optimise Zero Point Threshold in wad_exp Dec 14, 2023
Signed-off-by: Pascal Marco Caversaccio <pascal.caversaccio@hotmail.ch>
Signed-off-by: Pascal Marco Caversaccio <pascal.caversaccio@hotmail.ch>
Copy link
Owner

@pcaversaccio pcaversaccio left a comment

Choose a reason for hiding this comment

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

Thx @engn33r!

@pcaversaccio pcaversaccio changed the title ⚡️ Optimise Zero Point Threshold in wad_exp ⚡️ Optimise the Zero Point Threshold in wad_exp Dec 14, 2023
@pcaversaccio pcaversaccio merged commit 1e30d03 into pcaversaccio:main Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

optimisation ⚡️ Code optimisations (e.g. gas improvements)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants