Skip to content

Clean up unnecessary splatting #4383

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

Open
wants to merge 53 commits into
base: main
Choose a base branch
from
Open

Conversation

simone-silvestri
Copy link
Collaborator

We still have a performance issue related to the implicit solver. It is very difficult to diagnose. I tried to remove splatting where possible and the Val(tracer_index) to see if that creates weird problems. This PR is open just to see if the tests speed up with this change.

@glwagner
Copy link
Member

might be worthing lookikng at this commit

8dfb763

@navidcy navidcy added the performance 🏍️ So we can get the wrong answer even faster label Apr 10, 2025
@simone-silvestri simone-silvestri changed the title Try fixing performance Clean up unnecessary splatting Apr 17, 2025
@simone-silvestri
Copy link
Collaborator Author

Ok, it seems that we have no performance hit, I was mistaken. However, this PR removes a lot of unnecessary splatting making the code clearer, so I would still consider merging it.

@simone-silvestri
Copy link
Collaborator Author

(The @muladd creeped into here so I will remove it before merging)

@simone-silvestri simone-silvestri added benchmark performance runs preconfigured benchamarks and spits out timing cleanup 🧹 Paying off technical debt and removed performance 🏍️ So we can get the wrong answer even faster labels Apr 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark performance runs preconfigured benchamarks and spits out timing cleanup 🧹 Paying off technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants