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

Speed up for classification by refactoring solution interface #133

Merged
merged 13 commits into from
Jan 2, 2024

Conversation

jkosata
Copy link
Member

@jkosata jkosata commented Dec 28, 2023

get_single_solution is supposed to be a user-friendly method but is terribly inefficient for internal work, which has been an issue before (#86 , #130 ) . Rather, we want to pass vectors of variables + varied parameters around, in a fixed order.

Here:

  • internal constructions of OrderedDict removed (still works as a UI method)
  • Jacobians multiple-dispatched to accept both OrderedDict and a vector of variables + varied parameters
  • classification refactored to directly call transform_solutions

_classify_default for 1000 solutions of 9 branches, before:
2023-12-28_22-17

after:
2023-12-28_22-19

@jkosata jkosata requested a review from oameye December 28, 2023 21:38
@oameye
Copy link
Member

oameye commented Dec 29, 2023

Oeoeo 3x speedup is already very nice! I will review the code today.

@oameye
Copy link
Member

oameye commented Dec 29, 2023

i don't see the bug. I think it is better you do it.

@jkosata
Copy link
Member Author

jkosata commented Dec 30, 2023

I'm not seeing this, for example with the parametron.jl test, everything runs fine
(there are other issues I'm on though)

@oameye oameye marked this pull request as ready for review January 1, 2024 13:02
Copy link
Member

@oameye oameye left a comment

Choose a reason for hiding this comment

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

I think this is a major improvement! Very nice!

One quick question. What is the idea about batching the threading instead of just using @threads macro of all indices over the solution matrix?

@oameye oameye changed the title Compiled functions Speed up for classification by refactoring solution interface Jan 1, 2024
@oameye oameye added the performance Regarding performance of the package label Jan 1, 2024
@oameye oameye linked an issue Jan 1, 2024 that may be closed by this pull request
@oameye
Copy link
Member

oameye commented Jan 1, 2024

@jkosata I will leave merging up to you. If you want you can tag 0.7.3 in the Project.toml and comment @JuliaRegistrator register in the commit on the master branch (see example).

@oameye
Copy link
Member

oameye commented Jan 2, 2024

Before you Tag the master branch to the Julia register, we could also merge the method keyword from branch method-keyword.

@oameye oameye merged commit b39eefc into master Jan 2, 2024
2 checks passed
@oameye oameye deleted the compiled_functions branch January 2, 2024 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Regarding performance of the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

get_single_solution use in internals
2 participants