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

Modelling issues #2125

Closed
lanzagar opened this issue Mar 21, 2017 · 5 comments
Closed

Modelling issues #2125

lanzagar opened this issue Mar 21, 2017 · 5 comments

Comments

@lanzagar
Copy link
Contributor

Orange version

3.4.1

Issues
  1. 'modelling' is missing in Orange/__init__.py and needs to be explicitly imported (unlike other submodules)
  2. Some learners are not imported in Orange/modelling/__init__.py (i.e. Constant and SGD)
  3. Other imports use *, but the modules do not define __all__ so things are pulled into modelling that should not be
  4. Not sure what the plan is for the linear.py submodule, but if it will only contain SGD it could be renamed to e.g. sgd.py?
  5. SGDLearner() can't be called without parameters, because of _change_kwargs. I would like/expect defaults to be used when not providing specific losses etc.
  6. SGDLearner uses Learner.__call__ instead of SklLearner.__call__ so the attribute used_vals is not set, and params does not exist. This is a problem at least in [FIX] Widget Logistic Regression: can handle unused values #2116, but could have other implications. I think probability predictions are not expanded correctly (see SklModelClassification.__call__ or compare with e.g. logreg on first half of iris)!
@lanzagar
Copy link
Contributor Author

@pavlin-policar could you take a look?

@pavlin-policar
Copy link
Collaborator

Yes, sure thing. I'll get right on it.

As far as 4. is concerned, I don't know what would make more sense, because the classification and regression modules are not consistent with each other. In regression, we have a linear.py file that contains everything from linear regression to sgd. And there's also a lbfgs version of linear regression. On the other hand, in the classification module, logistic regression and sgd are in separate files, but they are both essentially regression... Having one file called linear.py made more sense to me, but it is not consistent. Logistic regression and linear regression will not be merged, so it may make more sense to call it sgd.

@lanzagar
Copy link
Contributor Author

I think this has the lowest priority and can wait for some discussion and consensus. I agree it makes sense in regression where it contains multiple classes. If it will remain with only one class in modelling, I think it makes less sense. But it is only a name after all, so several choices are good enough.

@ajdapretnar
Copy link
Contributor

@lanzagar Can this be considered resolved?

@lanzagar
Copy link
Contributor Author

Sure, why not.

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

No branches or pull requests

3 participants