Skip to content

Conversation

@dstrain115
Copy link
Collaborator

  • This PR changes ParamResolvers to only use string keys.
  • Previously, ParamResolver keys could be strings or symbols.
  • If a symbol is detected, the ParamResolver will remake the dictionary using only strings.
  • This simplifies the logic and improves performance since sympy Symbols can be slower to create and hash.

Performance implications:

  • Creation speed is unchanged unless you have symbols as keys, then creation takes about twice as long. (For 1000 parameters, this is a change from about 0.6ms to 1.3ms).
  • Lookup speed is now about ~120-130ns
  • Before it was: -- ~200ns for string lookup with string keys
    -- ~250ns for symbol lookup with string keys
    -- ~850ns for string lookup with symbol keys
    -- ~350ns for symbol lookup with symbol keys

- This PR changes ParamResolvers to only use string keys.
- Previously, ParamResolver keys could be strings or symbols.
- If a symbol is detected, the ParamResolver will remake the dictionary
using only strings.
- This simplifies the logic and improves performance since sympy Symbols
can be slower to create and hash.

Performance implications:
- Creation speed is unchanged unless you have symbols as keys, then
creation takes about twice as long.  (For 1000 parameters, this is
a change from about 0.6ms to 1.3ms).
- Lookup speed is now about ~120-130ns
- Before it was:
-- ~200ns for string lookup with string keys
-- ~250ns for symbol lookup with string keys
-- ~850ns for string lookup with symbol keys
-- ~350ns for symbol lookup with symbol keys
@dstrain115 dstrain115 requested review from a team and vtomole as code owners October 22, 2025 14:41
@dstrain115 dstrain115 requested a review from maffoo October 22, 2025 14:41
@github-actions github-actions bot added the size: S 10< lines changed <50 label Oct 22, 2025
@github-actions github-actions bot added size: M 50< lines changed <250 and removed size: S 10< lines changed <50 labels Oct 22, 2025
@dstrain115 dstrain115 marked this pull request as draft October 22, 2025 16:39
@github-actions github-actions bot added size: S 10< lines changed <50 and removed size: M 50< lines changed <250 labels Oct 22, 2025
@codecov
Copy link

codecov bot commented Oct 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.38%. Comparing base (036f969) to head (ed31386).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7714   +/-   ##
=======================================
  Coverage   99.38%   99.38%           
=======================================
  Files        1091     1091           
  Lines       97813    97820    +7     
=======================================
+ Hits        97212    97219    +7     
  Misses        601      601           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dstrain115 dstrain115 marked this pull request as ready for review October 22, 2025 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: S 10< lines changed <50

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant