-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
replace awkward state changes with free functions #4999
Comments
I agree it is one way to solve the problem without having to twist the put/get/run api too much. For me this would be a temporary solution, where the proper solution would be to refactor/redesign the classes such that these stunts are not necessary. On the other hand, I am pretty sure that there will always be corner cases. So +1 overall from my side. |
Hi! I'd like to help with this but i'm not sure how to get started as this would be my first contribution on a git open source project. |
As we find more and more edge cases of the parameter framework the API has become increasingly awkward. For example,
DeepAutoencoder::convert_to_neural_network(std::shared_ptr<NeuralLayer> output_layer, float64_t sigma)
can be replaced with a class function without parameters by adding the output_layer and sigma to the state, but then the user has to alter the state of the object for all kind of specific functions, and it becomes hard to keep track of.Another case is to calculate the out of bag error:
Why should the machine have to keep the evaluation metric it in its state?
With that in mind why don't we replace some of these class functions with free functions, that try to down_cast the base class and dispatch the call?
or for multiple types (from most specialised, could cause bugs if not careful):
These things will undo some of the work towards reducing the SWIG wrapper LoC but at least the API becomes cleaner (imo), and the boiler plate code they add is minimal compared to a class.
The text was updated successfully, but these errors were encountered: