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

Remove SVM from interfaces #5085

Open
gf712 opened this issue Jun 29, 2020 · 11 comments
Open

Remove SVM from interfaces #5085

gf712 opened this issue Jun 29, 2020 · 11 comments

Comments

@gf712
Copy link
Member

gf712 commented Jun 29, 2020

Currently we expose SVM to interfaces, but this is somewhat redundant as we expose Machine (and SVM is a Machine derived class). This was just a hack to make sure that only SVM types can be passed with put to objects expecting an SVM parameter. But now we can test at runtime if the object being put can be casted to the expected type:

SG_ADD((std::shared_ptr<Machine>*)&estimate, "estimate", "Plugin Estimate.", ParameterProperties::CONSTRAIN,
SG_CONSTRAINT(castable<PluginEstimate>()));

So all Machine which expect an SVM parameter (e.g. MKLClassification) should have this castable check and register the svm parameter as a Machine. This will avoid having to cast machine to svm when passing it to things like MKLClassifcation:

svm = sg.create_machine("MKLClassification", svm=sg.as_svm(libsvm),
interleaved_optimization=False, kernel=kernel)

@karlnapf the only issue will be how does the user have access to compute_svm_primal_objective and compute_svm_dual_objective? Maybe SVM shouldn't be removed from swig, but classes like MKL should register SVM as Machine? That way we avoid having to cast the Machine object when it is just passed as an argument in put?

@karlnapf
Copy link
Member

@karlnapf the only issue will be how does the user have access to compute_svm_primal_objective and compute_svm_dual_objective? Maybe SVM shouldn't be removed from swig, but classes like MKL should register SVM as Machine? That way we avoid having to cast the Machine object when it is just passed as an argument in put?

That is no problem, we can expose that lazily via get and functions that take no arguments (I think).

Note we can also remove the as_* functions in the swig interface

@gf712
Copy link
Member Author

gf712 commented Jun 30, 2020

That is no problem, we can expose that lazily via get and functions that take no arguments (I think).

The issue is that those functions require Labels and as we are making Machine stateless wrt to Labels we will have to pass labels as an argument to these functions? That would not be possible with the current get/put functionality, but could be done with a free function?

@karlnapf
Copy link
Member

ah! crap! :)
TBH I don't think it is such a problem.... I would actually just not expose it for now.
But these cases require a solution eventually ...

@gf712
Copy link
Member Author

gf712 commented Jun 30, 2020

ah! crap! :)
TBH I don't think it is such a problem.... I would actually just not expose it for now.
But these cases require a solution eventually ...

Would it be possible to just compute those values when apply is called or is that too much unnecessary overhead?

@karlnapf
Copy link
Member

they are computed anyways when fitting the model.
But how would you compute them in apply? Would you store them? Dont think that is any different from storing the labels

@gf712
Copy link
Member Author

gf712 commented Jun 30, 2020

they are computed anyways when fitting the model.
But how would you compute them in apply? Would you store them? Dont think that is any different from storing the labels

The only use case I found of this in the library is in a notebook, where we get the objective values from training labels, not from new labels. But I guess if you want to compute it for new labels then this should be a metric class with an evaluate method (I know it might not make a lot of sense to call this a metric), or a free function compute_svm_primal_objective(svm, labels)?

@karlnapf
Copy link
Member

Just realised, the solution is an observer

@karlnapf
Copy link
Member

@geektoni

@geektoni
Copy link
Contributor

Just realised, the solution is an observer

Just to understand better, do you need to expose just computed values or directly a function which needs to be called by the user? :)

@gf712
Copy link
Member Author

gf712 commented Jun 30, 2020

@karlnapf I guess the idea is that these values are observed and you emit the observation when calling train/apply? I guess then it would be the values that are exposed, because a function would require updating the labels in the SVM, which would make it stateful wrt to labels again.

@karlnapf
Copy link
Member

The observer would know that the user wants to access the objective values. It then simply stores them. During training, a needs to be omitted to the observer, which then checks whether it wants to store it, and if so, it calls it. Alternatively (as I think the objective is always computed during the training loop), can just emit it always...
I think this doesnt have priority though, so it shouldnt block us from removing svm from the interfaces

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants