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

Add unify keyword and default if to true #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Suavesito-Olimpiada
Copy link
Contributor

fix #48

@Suavesito-Olimpiada
Copy link
Contributor Author

Still to add tests and discuss the proper algorithm to unify boxes.

@lucaferranti
Copy link
Member

Out of curiosity, do you know how much computational overhead this introduces? Just wondering which one would be a better default value.

Here a simple MWE that might be good for benchmarking

julia> f(x) = x^2 - 2x + 1
f (generic function with 1 method)

julia> minval, minimisers = minimise(f, 0..2, tol=1e-6)

this example should return 2990 minimisers, so it might be a good small but not too small example for benchmarking

@lucaferranti
Copy link
Member

Ah, you answered my questions while I was writing that 😄 Amazing! sorry for the noise

@Suavesito-Olimpiada
Copy link
Contributor Author

Don't worry, happy to help. :D

@dpsanders
Copy link
Member

At a first glance this looks great, thanks! Could you please add some tests?

@Suavesito-Olimpiada
Copy link
Contributor Author

Sure, I'll add test. I just want to know what do you think about the "problem" of using union-find that I described in the comment in the original issue.

return disjoint_sets[i]
end

function add_set!(disjoint_sets, weights_sets, i, j)
Copy link
Member

Choose a reason for hiding this comment

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

What is weights_sets for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is an optimizations for union-find to make the union and find operations almost constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the fastest asymtotically variant known of union-find. The algorithm is described here.

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.

Unify boxes in result by default?
3 participants