On Sun, Sep 30, 2012 at 8:28 AM, Andreas Mueller <[email protected]> wrote: > > >> Why do you want to rewrite the predict code, which seems to be already >> working? >> (Doesn't this further divergence from the libsvm code base just >> increase the sklearn maintenance burden?) >> >> The key thing seems to be how heavily patched is the svm.cpp already? >> If it's completely rewritten, then trying to work with the original >> project is silly, but I don't think it is. It seems like there are a >> few things: >> >> (1) the use of PREFIX and the _DENSE_REP ifdef, and the extra >> double-include file that drives that mechanism >> >> (2) changing the upper_bound in solution_info to a buffer of len 2 >> instead of 2 different variables >> >> (3) what looks like algorithmic changes around line 1600 that I don't >> understand >> >> I could certainly be wrong, but these things still look maintainable >> as a patch set. Why do you want to break further away from the libsvm >> trunk, rather than refactor things to be, if anything, *more* >> compatible with it? >> > I think the idea is that a lot of the code could be made more accessible > and shorter by rewriting it using cython and numpy. >
I can appreciate that, but let's come back to the original question -- which was what to do with this PR? There are at least two top-level questions that have come up here: 1. whether to merge my PR [roughly as-is] into sklearn 2. whether to create a libsvm fork that has sklearn's nice features built into it. 2. b) If the answer here is yes, then how should sklearn be re-factored to use that fork. Let's come back to (2) later when someone has the energy for it regardless of the choice regarding 1. My PR does not represent a significant divergence from the libsvm source compared to what has already been done. So can we come back to (1)? What are people's thoughts on my PR? It passes tests and respects code conventions AFAIK. I've addressed the point regarding the warning vs. exception. https://github.com/scikit-learn/scikit-learn/pull/1184 - James ------------------------------------------------------------------------------ Don't let slow site performance ruin your business. Deploy New Relic APM Deploy New Relic app performance management and know exactly what is happening inside your Ruby, Python, PHP, Java, and .NET app Try New Relic at no cost today and get our sweet Data Nerd shirt too! http://p.sf.net/sfu/newrelic-dev2dev _______________________________________________ Scikit-learn-general mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/scikit-learn-general
