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: improve performance of Autodiff #796

Merged
merged 11 commits into from Jan 10, 2022
Merged

perf: improve performance of Autodiff #796

merged 11 commits into from Jan 10, 2022

Conversation

joshsunshine
Copy link
Member

Description

Autodiff gradient tests ran during every diagram generation so this PR turns those tests off. This PR also converts the recursive graph traversal into iterative traversals to avoid stack overflows. Finally, this PR combines multiple graph traversals to clear the cache into a single traversal.

Related issue/PR: # (issue/PR)
#652
This issue is not complete, but the simple changes do improve the performance of Autodiff.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • New and existing tests pass locally using yarn test
  • I ran yarn docs and there were no errors when generating the HTML site
  • My code follows the style guidelines of this project (e.g.: no ESLint warnings)

Open questions

Questions that require more discussion or to be addressed in future development:

@codecov
Copy link

codecov bot commented Jan 10, 2022

Codecov Report

Merging #796 (3083bc8) into main (b697a80) will decrease coverage by 0.50%.
The diff coverage is 49.60%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #796      +/-   ##
==========================================
- Coverage   65.72%   65.22%   -0.51%     
==========================================
  Files          59       59              
  Lines        7980     8040      +60     
  Branches     1405     1413       +8     
==========================================
- Hits         5245     5244       -1     
- Misses       2726     2787      +61     
  Partials        9        9              
Impacted Files Coverage Δ
packages/core/src/types/ad.ts 100.00% <ø> (ø)
packages/core/src/engine/Autodiff.ts 55.00% <44.73%> (-3.00%) ⬇️
packages/core/src/utils/Util.ts 34.48% <90.24%> (+9.18%) ⬆️
packages/core/src/utils/OtherUtils.ts 62.40% <0.00%> (-9.61%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b697a80...3083bc8. Read the comment docs.

@cloudflare-pages
Copy link

cloudflare-pages bot commented Jan 10, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3083bc8
Status: ✅  Deploy successful!
Preview URL: https://2a9599c4.penrose-panes.pages.dev

View logs

@wodeni wodeni merged commit 8bca6db into main Jan 10, 2022
@wodeni wodeni deleted the s-formula branch January 10, 2022 22:28
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.

None yet

3 participants