naywhayare changed the topic of #mlpack to: http://www.mlpack.org/ -- We don't respond instantly... but we will respond. Give it a few minutes. Or hours. -- Channel logs: http://www.mlpack.org/irc/
sumedh_ has quit [Ping timeout: 264 seconds]
udit_s has joined #mlpack
udit_s has joined #mlpack
udit_s has quit [Quit: Leaving]
udit_s has joined #mlpack
udit_s has quit [Quit: Leaving]
Anand has joined #mlpack
Anand has quit [Ping timeout: 246 seconds]
Anand has joined #mlpack
< Anand>
Marcus : I added the print function to log. You can run scikit/nbc and see if it is good enough
< Anand>
Also, any pointers on bootstrapping?
Anand has quit [Ping timeout: 246 seconds]
Anand has joined #mlpack
Anand has quit [Ping timeout: 246 seconds]
< marcus_zoq>
:q
< jenkins-mlpack>
Starting build #2019 for job mlpack - svn checkin test (previous build: SUCCESS)
< jenkins-mlpack>
* saxena.udit: Adaboost design issues, to be discussed, then changed later on
< jenkins-mlpack>
* saxena.udit: Minor updates
udit_s has joined #mlpack
andrewmw94 has joined #mlpack
udit_s has quit [Quit: Leaving]
sumedh__ has joined #mlpack
sumedh_ has quit [Ping timeout: 264 seconds]
< naywhayare>
sumedh__: could you document the termination policies and update rules that you've written more thoroughly please? they are hard to debug because I don't know what a lot of the variables are for or how they are intended to be used...
< naywhayare>
if you could add, for each set of update rules, a reference to the paper it came from (and where in the paper it came from), that would be helpful too
< sumedh__>
Okay I will do that... I thought I will do that once the project is ready...
< naywhayare>
by that time you may have forgotten, but ok :)
< naywhayare>
I had to make a small change to svd_incremental_learning.hpp; I changed
< sumedh__>
ohh yes.. I solved that in my test svn... I forgot to make the changes in the main svn... sorry for that...
< naywhayare>
it's ok, it was an easy fix :)
< naywhayare>
I just wanted to check that row(i) was the correct thing to add
< naywhayare>
and not row(someOtherVariable)
< sumedh__>
no no... row(i) is correct...
< naywhayare>
ok, great
< sumedh__>
so regularization seems correct??
< naywhayare>
I don't have an answer quite yet; I am still trying some things
< naywhayare>
but the implementation appears to be correct to me
< sumedh__>
ohh okay... I tried with different parameters but regularization is never giving a better answer...
< naywhayare>
so I think the issue might actually be in the termination policy, but I am still testing things so I'm not sure yet
< naywhayare>
let me tell you what I've done...
< naywhayare>
I noticed that in ValidationRMSETermination, IsConverged() is getting called way more than Step()
< sumedh__>
as the regularization decreases it tends to the value obtained from no regularization...
< naywhayare>
this is because in IncompleteIncrementalTermination, the IsConverged() function doesn't check that iteration % incrementalIndex == 0
< naywhayare>
so the ValidationRMSETermination::IsConverged() check happens every time HUpdate() and WUpdate() are called -- which is once per user update
< naywhayare>
if three consecutive users increase the RMSE, it terminates
< naywhayare>
but I think that should be, if three consecutive passes over all users increases the RMSE, it terminates
< naywhayare>
so the change I made changed IncompleteIncrementalTermination::IsConverged() to have the following body:
< naywhayare>
if (iteration % incrementalIndex == 0)
< naywhayare>
return t_policy.IsConverged(W, H);
< naywhayare>
else
< naywhayare>
return false;
< sumedh__>
ohh yes... you are right...
< naywhayare>
I am running the test with various parameters to see what the effect is
< sumedh__>
the condition should be added to both the step and IsConverged...
< naywhayare>
IncompleteIncrementalTermination::Step() already has the condition, so no need to add it there
< sumedh__>
thats a very stupid mistake I have done...
< naywhayare>
it's ok, not a problem :)
< naywhayare>
I was thinking, maybe we could combine Step() and IsConverged() into one function? IsConverged(V, W, H)
< naywhayare>
what do you think?
< naywhayare>
it seems like AMF always calls Step() and then IsConverged()
< sumedh__>
hummm... in current scenario there is no problem if we combine them... but I think the name of the combined function should be more descriptive...
< sumedh__>
like StepAndCheckConvergence(..) or something like that...
< naywhayare>
I dunno, I think IsConverged() is fine
< naywhayare>
I think it's kind of implicit that the termination policy has to do some updating
< sumedh__>
now regularization making any positive effect??
< naywhayare>
it seems like it is helping
< naywhayare>
but I want to try several sets of parameters
< naywhayare>
to make sure that I didn't just happen to pick a lucky parameter set that works
< sumedh__>
yes I will do that... right now I am implementing the Complete Incremental Learning... tonight I will make a commit with new test for incomplete incremental learning and implementation of complete incremental learning...
< naywhayare>
it seems like when I set regularization to small values like 1e-5 or lower it has very little or no effect
< naywhayare>
I am having better results with 0.01
< naywhayare>
it looks like in the paper they used 0.05 for the Movielens dataset and 0.005 for the Netflix dataset
< sumedh__>
great... so regularization test can be written now...
< naywhayare>
ok, let me explain what I've tried
< naywhayare>
I tried a couple different learning rates: 0.001, 0.0011, 0.0013, 0.003 (too large), 0.0005 (quite slow)
< naywhayare>
and for each of those I tried some different regularization parameters: 0.1, 0.01, 0.001, 1e-5
< naywhayare>
then, remembering that regularization is meant to prevent overfitting, I decided to add some noise to the training set
< naywhayare>
hoping that maybe SVD without regularization would overfit on the noise a little bit
< naywhayare>
and that regularization would help it generalize
< naywhayare>
but I didn't see much of a change in the results; just like the non-noisy case, regularization seemed to perform somewhat better
< sumedh__>
to which dataset you added noise??
< sumedh__>
cause we can assume that MovieLens dataset would itself be noisy...
< naywhayare>
oh, maybe that is why it didn't do anything then :)
< naywhayare>
and yeah, I added noise to the MovieLens dataset
< sumedh__>
MovieLens is a real dataset so it will be noisy... the dataset is collected from their online site... as the souce states...
< sumedh__>
*source
< sumedh__>
and is their a function to add noise in MLPACK??
< naywhayare>
no, I just iterated over the nonzero entries in the data matrix and added math::Random() - 0.5
< naywhayare>
have you tried the test with the changed IsConverged() function?
< naywhayare>
it should work with only that change
< naywhayare>
but there is one other thing I noticed... I am printing the RMSE value at the end of each call to Step()
< naywhayare>
for a step size of 0.001 and regularization parameters of 0.01, it gets down to 0.906469, but the returned RMSE is 0.952476
< sumedh__>
ohh... then I guess there is bug in matrix storing I am doing.... I will check that right now...
< naywhayare>
you can try and reproduce it, and see if you get the same behavior
< sumedh__>
its on 0.90 for long long time ....
< naywhayare>
yeah, I set the maximum iterations to 1000
< naywhayare>
I got better convergence for a learning rate of 0.0011
< naywhayare>
you could also set reverseStepTolerance to one less than it is by default; maybe that would help too
< sumedh__>
its working fine for me... yes but on test svn... let me check my committed code...
Anand has joined #mlpack
udit_s has joined #mlpack
< marcus_zoq>
Anand: Hello, I've looked at the output code, maybe you can modify the code in a way, so that we can easily compare the results of the different libraries? Right now we need to search for the corresponding result for a specific metric, right?
< udit_s>
naywhayare: Hello. Have you looked at the email and the code ?
< Anand>
Marcus : That can be done only when we have all the metrics for different libraries stored somewhere, right? The code currently deals in only one library. We might have to use the database for comparing
andrewmw94 has quit [Quit: Leaving.]
andrewmw94 has joined #mlpack
< marcus_zoq>
Anand: Actually what I did is: store the results into a data structure and if the user asked for, save the data into the database. So, I think we can do the same with the metrics?
< Anand>
Which data structure are you talking about? And where do you compare the results?
< Anand>
Marcus : If we have a class which can access all metrics of all libraries, we can do this.
< marcus_zoq>
Anand: In run_benchmark.py is a datastructure called dataMatrix which stores all timing results. We can add a similar structure for the metrics. In this case the RunMetric method needs to return the results.
< Anand>
Ok. We can easily return an array containing the results from the current code.
< naywhayare>
udit_s: I glanced at it but I haven't addressed it completely yet
< Anand>
And I guess some modification will be required in the "if 'metric' in tasks:" part of the code?
< naywhayare>
yesterday, I went through decision_stump_main.cpp and commented it and gave lots of output in the PROGRAM_INFO() macro. Could you do the same for perceptron_main.cpp? you can use my changes as an example
< naywhayare>
good documentation there is very important, because that is all most users will ever see; you can assume they kind of know what a perceptron is, but probably not more
< naywhayare>
anyway, I will look through the AdaBoost code and answer your email...
< udit_s>
naywhayare: okay. sure.
< sumedh__>
naywhayare: there was a bug in my committed copy of validation_rmse_termination.hpp... very sorry for that...
< sumedh__>
solved it ...
< naywhayare>
sumedh__: not a problem, good to hear you fixed it
< naywhayare>
udit_s: excellent, thank you
< sumedh__>
I am running test again to see if it is fixed...
< sumedh__>
naywhayare: and another bug in amf_impl.hpp... we were returning the sqrt of Index()... old implementation...
< naywhayare>
udit_s: also, don't worry about giving support for weighted datasets or anything in the perceptron_main program. the idea is just to expose a simple interface to a casual user that has most of the simple functionality
< Anand>
Marcus : We can call instance.RunMetrics there for all instances (libraries?) which will return an array and then we can compare the metrics as we like. What do you say?
< udit_s>
naywhayare: the function for weighted dataset manipulation is to be used in the adaboost function. This will modularize the modifyData() function, allowing for more generalized weak learners
< Anand>
Marcus : We do this in the "if 'metric' in tasks:" condition .
< udit_s>
naywhayare: hence their implementation in perceptron and decision_stump
< marcus_zoq>
Anand: Yeah sounds good, and yes instances = libraries. Maybe we should rename this :)
< Anand>
Ok, I will do that. And how do you want to compare them after getting metrics for all libraries?
< marcus_zoq>
Anand: Ony way is to use the existing infrastructure (printTable) and use the metric names instead of the datasets?
< naywhayare>
udit_s: I don't completely understand, but I finished an email response; can you take a look at that and tell me what you think? that may change things a little
< Anand>
metric names instead of the datasets? where?
< Anand>
Marcus : we can use printTable to just see the figures for a particular metric (for all libraries) together
< Anand>
Marcus : Other thing that we can do later is to compare them graphically
< marcus_zoq>
Anand: If you use this way you need to return the results of the metrics and the metric name.
< Anand>
Marcus : Yes, exactly!
< Anand>
We will do this
< Anand>
Marcus : Actually, this is now heading towards bootstrapping!
Anand has quit [Ping timeout: 246 seconds]
< udit_s>
naywhayare: okay. I think I get what you mean. But doesn't this mean that apart from a few lines of code (which copies/takes the parameters from an already learned weak learner, and say, modifies the weights/distribution), the new constructor will essentially be the same old constructor ?
< naywhayare>
yeah, so it may be useful to create a private Train() function that the constructor calls
< naywhayare>
or that multiple constructors call
< naywhayare>
that way you can increase code reuse
< udit_s>
Yeah, that's what I meant in the mail. I was worried about code reuse.
< naywhayare>
yeah, you should be able to still do this and also ensure that a trained classifier is built in each constructor
< udit_s>
and in the first few lines, I'll be modifying the dataset using the weights. Okay, I'll get this done by tomorrow on the decision stump and the perceptron. Should I consider working on the NBC too ? Or shall I work on that in the end, when I've successfully implemented Adaboost on Decision Stump and Perceptron ?
< naywhayare>
yeah, let's save NBC for later
< udit_s>
okay then...
udit_s has quit [Quit: Leaving]
< andrewmw94>
naywhayare: I found a bug where I have a function that expects a RectangleTree<...>& and is passed a pointer given by tree.Child(i) . Though I found it now, I'm confused about why the compiler doesn't give me an error. It doesn't seem to automatically cast the pointer into a reference. Anything I should know about?
< naywhayare>
I thought tree.Child(i) returned a reference
< naywhayare>
where was the bug? I assume I can still find it in svn
< andrewmw94>
Sorry, I got dinner. The bug is rectangle_tree_test.cpp
< andrewmw94>
the checkSync() function calls itself recursively
< andrewmw94>
my line numbers are messed up, but it's somewhere around line 150
< andrewmw94>
searching for checkSync is probably faster though
< naywhayare>
I see the code you're talking about
< naywhayare>
but I'm booked for the rest of the night... I will take a look tomorrow morning
< naywhayare>
and see if I can concoct some kind of reasonable explanation