sklearn adapter

[yangarbiter]

#20

[codecov-io]

Current coverage is 84.94% (diff: 82.85%)

Merging #70 into master will decrease coverage by 0.20%

@@             master        #70   diff @@
==========================================
  Files            19         20     +1   
  Lines           566        598    +32   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            482        508    +26   
- Misses           84         90     +6   
  Partials          0          0          

Powered by Codecov. Last update 345a36e...a9736d0

[yangarbiter]

@hsuantien
I am a little unsure about the design of the interface ContinuousModel.
It seems a little unclear to new users and not sure the sklearnAdapter should belong to Model or ContinuousModel
I would suggest the we remove ContinuousModel and move the predict_real to the Model interface.
It the algorithm didn't support continuous prediction, we should just raise NotImplementedError or create a NotSupportedError.
How do you think.

[hsuantien]

I personally view ContinuousModel as Model with additional capability. It
seems to me that mixing the two together actually makes it harder to
specify the correct one for active learning algorithms, right?

If sklearn models generally support continuous prediction, maybe we can
just put the adapter under ContinuousModel (or have a SKContinuousAdapter).

Thanks.

--Hsuan-Tien

On Thu, Nov 10, 2016 at 11:31 AM, yangarbiter [email protected]
wrote:

@hsuantien https://github.com/hsuantien
I am a little unsure about the design of the interface ContinuousModel.
It seems a little unclear to new users and not sure the sklearnAdapter
should belong to Model or ContinuousModel
I would suggest the we remove ContinuousModel and move the predict_real to
the Model interface.
It the algorithm didn't support continuous prediction, we should just
raise NotImplementedError or create a NotSupportedError.
How do you think.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#70 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGWRdacEX7nqj42U-YzBimZez00Y4_Dcks5q8pAugaJpZM4KpNqd
.

---

Hsuan-Tien Lin [email protected]

http://www.csie.ntu.edu.tw/~htlin

Associate Professor
Dept. of Computer Science and Information Engineering
& Graduate Institute of Networking and Multimedia

National Taiwan University

[yangarbiter]

What I am thinking is not to specify the inputed model through its inherited interface but through whether a particular method is implemented.
If we called some model.predict_real and it isn't support, we can just raise an error and the user should know that predict_real should be implemented.
At least it seems that scikit-learn is doing it this way.

[iamyuanchung]

As sklearn is one of the most important dependencies of libact, and most people are more familiar with it compared to other machine learning packages, I think it would be better to follow sklearn's design - to raise an error to notify the users to implement model.predict_real.

[iamyuanchung]

Agree.