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

Refactor classification to have additional keyword arguments #147

Closed
wants to merge 3 commits into from

Conversation

oameye
Copy link
Member

@oameye oameye commented Jan 8, 2024

No description provided.

@oameye oameye added the new feature New feature or request label Jan 8, 2024
@jkosata
Copy link
Member

jkosata commented Jan 13, 2024

Why is this needed? The current paradigm is that whenever solutions are manipulated, a function is constructed which takes the variables in a fixed order. This function is constructed internally (hence no kwargs). You can also construct it yourself, but in this case why kwargs if these cannot change?

@oameye
Copy link
Member Author

oameye commented Jan 13, 2024

I need it for the classification of the parametrons. I classify the solutions through the normal transformation matrix which i need to give to the classification function.
With kwarg I do not have to change the classification function every time I change the topology of the network.
I think it is a nice feature. But if you think it can affect performance?

@jkosata
Copy link
Member

jkosata commented Jan 13, 2024

I checked and kwargs should have the same performance (not sure though).
Why not just define your function with the matrix you need and pass it to classify_solutions! ? Having a construct where a constant kwarg is fed into a function you yourself just defined seems redundant.

@jkosata
Copy link
Member

jkosata commented Jan 13, 2024

Ad performance: I checked a simple case but there have been issues with splatting (...) before (https://discourse.julialang.org/t/why-is-the-splat-operator-slow/77928) , I think this construct just opens more ways to mess up

@oameye
Copy link
Member Author

oameye commented Jan 22, 2024

Closed in favor of using anonymous functions, e.g.,
classify_solutions!(result_ωλ, soln -> is_symmetric(soln; sym_basis=sym_basis), "symmetric")

@oameye oameye closed this Jan 22, 2024
@oameye oameye deleted the kwarg_classification branch February 19, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants