CogComp / lbjava

Learning Based Java (LBJava)
http://cogcomp.cs.illinois.edu/page/software_view/LBJava
Other
13 stars 17 forks source link

WIP: added adagrad class and unit test #13

Closed yj14n9xyz closed 8 years ago

yj14n9xyz commented 8 years ago

@christos-c I have added both AdaGrad class and unit test.

However, I couldn't run any of the existing tests because of some errors. I am not sure whether it's due to my set up or there is an issue in lbjava. I have run the unit test successfully in my own project.

Thanks.

christos-c commented 8 years ago

Thanks @yimingjiang. As you can see the tests pass in semaphore (you need to follow the instructions in README to get the test to run).

My only concern is that the unit test you added is too trivial. It tests that the AdaGrad class works and follows the Learner API but it doesn't actually test the algorithm itself. Do you think you can add another, more substantial test? @danyaljj @danr-ccg what do you think?

Also, I thought you had also added a RealTest class at some point?

danr-ccg commented 8 years ago

Yes, I think it would be good to add more substantial algorithmic tests. One option is to use the data we use in my CS446, where we compare several linear learning algorithms. We know what the expected results are (in fact, we have a “solution”, and can compare to it). This can be used, in fact, for several of our algorithms. We don’t do “average” Perceptron/Winnow/Adagrad in that problem set, but the data is still valid.

Thanks, Dan

From: Christos Christodoulopoulos [mailto:notifications@github.com] Sent: Tuesday, January 12, 2016 9:28 AM To: IllinoisCogComp/lbjava Cc: Roth, Dan Subject: Re: [lbjava] WIP: added adagrad class and unit test (#13)

Thanks @yimingjianghttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_yimingjiang&d=BQMCaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=EoEIPOcXriNKNTRQxPa5uv7hPypxUE6tE_RlGOluh-c&m=eEXepgtt_iMcvS91-LBOz0t_uEslC7B2DXQRhhCbFUk&s=pbaFVkApK14u5YpkTgv5BeH4594EODS8gMdoiwBPKSM&e=. As you can see the tests pass in semaphore (you need to follow the instructions in README to get the test to run).

My only concern is that the unit test you added is too trivial. It tests that the AdaGrad class works and follows the Learner API but it doesn't actually test the algorithm itself. Do you think you can add another, more substantial test? @danyaljjhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_danyaljj&d=BQMCaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=EoEIPOcXriNKNTRQxPa5uv7hPypxUE6tE_RlGOluh-c&m=eEXepgtt_iMcvS91-LBOz0t_uEslC7B2DXQRhhCbFUk&s=rC3eVGu9NkTYzzTe6Jc31-mzCF6u43_fQ2kV98hsQas&e= @danr-ccghttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_danr-2Dccg&d=BQMCaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=EoEIPOcXriNKNTRQxPa5uv7hPypxUE6tE_RlGOluh-c&m=eEXepgtt_iMcvS91-LBOz0t_uEslC7B2DXQRhhCbFUk&s=eo7VFDmSBQcZv70az0bVNxVCe3kktHB0r0XbnsE63dk&e= what do you think?

Also, I thought you had also added a RealTest class at some point?

— Reply to this email directly or view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_IllinoisCogComp_lbjava_pull_13-23issuecomment-2D170947026&d=BQMCaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=EoEIPOcXriNKNTRQxPa5uv7hPypxUE6tE_RlGOluh-c&m=eEXepgtt_iMcvS91-LBOz0t_uEslC7B2DXQRhhCbFUk&s=mZZEyy4kwxzDGKGDz_m1buvy8yvuQKNJUj-9J4uZtIQ&e=.

yj14n9xyz commented 8 years ago

@christos-c For AdaGrad unit test, learn is the place where AdaGrad algorithm is implemented. My thought is that feed in some examples via this API and see if the weights are correct according to the algorithm. The tests I wrote in my project is a complete process: reading from files, programmatically invoke AdaGrad classifier and check the weight. In both cases, the essence is to check the correctness of weight vector after seen some examples.

For TestReal class, it's my first time to actually contribute to the LBJava repo, so I want to add AdaGrad class related code first. Then I am going to merge also this TestReal class and some updates on documentation.

Last question for pull request, if I make more changes on branch add-adagrad-yiming, do I create a new pull request or use the current one?

Thanks for your help Christos!

yj14n9xyz commented 8 years ago

@danr-ccg @christos-c Professor Roth, the loss function used in CS 446 homework is hinge loss, however, in the AdaGrad I implemented, I used LMS as loss function, because SGD in LBJava used LMS as loss function.

Is it still desirable to use CS 446 HW data as tests? I doubt the results from two different loss functions will be identical. I have used http://archive.ics.uci.edu/ml/datasets/Airfoil+Self-Noise as test data now. The results from LBJava's SGD and AdaGrad are very very close in terms of RMS error.

Thanks !

danr-ccg commented 8 years ago

Hi Yiming,

Ok, I see. First, I think that it is important to have also Adagrad for Hinge loss, which is basically a modification of Perceptron, simply since it is likely will do better. Also, is it possible to use the new algorithm also with the Averaged case? I think that the data sets from CS446 should work well, although other data sets could also be good. The one you have used has very few features, and we definitely want to use also something that has a large number of features, since more of the interesting data sets have this property. My thinking is that it would be good to build a table of all the algorithms we have, and produce results on them, along with exact results on a small subset, and then we can use these two as tests for all our algorithms. Makes sense?

Thanks, Dan

From: Yiming Jiang [mailto:notifications@github.com] Sent: Tuesday, January 12, 2016 8:06 PM To: IllinoisCogComp/lbjava Cc: Roth, Dan Subject: Re: [lbjava] WIP: added adagrad class and unit test (#13)

@danr-ccghttps://urldefense.proofpoint.com/v2/url?u=https-3Agithub.com_danr-2Dccg&d=BQMCaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=EoEIPOcXriNKNTRQxPa5uv7hPypxUE6tE_RlGOluh-c&m=bqLD53fRz-777VQg0f3yqa5otQ3Up_pgY09fXeVM_yc&s=LgnX-fwmEujAyk4Mt3XfyNFv6dCdH3yG6MtveGraN58&e= @christos-chttps://urldefense.proofpoint.com/v2/url?u=https-3Agithub.com_christos-2Dc&d=BQMCaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=EoEIPOcXriNKNTRQxPa5uv7hPypxUE6tE_RlGOluh-c&m=bqLD53fRz-777VQg0f3yqa5otQ3Up_pgY09fXeVM_yc&s=FpQhD5guYUUM48KAHvaDP4-y_JGbvTX3mmTTB35kEPs&e= Professor Roth, the loss function used in CS 446 homework is hinge loss, however, in the AdaGrad I implemented, I used LMS as loss function, because SGD in LBJava used LMS as loss function.

Is it still desirable to use CS 446 HW data as tests? I doubt the results from two different loss functions will be identical. I have used http://archive.ics.uci.edu/ml/datasets/Airfoil+Self-Noisehttps://urldefense.proofpoint.com/v2/url?u=http-3A__archive.ics.uci.edu_ml_datasets_Airfoil-2BSelf-2DNoise&d=BQMCaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=EoEIPOcXriNKNTRQxPa5uv7hPypxUE6tE_RlGOluh-c&m=bqLD53fRz-777VQg0f3yqa5otQ3Up_pgY09fXeVM_yc&s=1KXVVqQJ9IBiVdwAvt_zThW_jIrPY_fTzqkumi0Xldo&e= as test data now. The results from LBJava's SGD and AdaGrad are very very close in terms of RMS error.

Thanks !

— Reply to this email directly or view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_IllinoisCogComp_lbjava_pull_13-23issuecomment-2D171129999&d=BQMCaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=EoEIPOcXriNKNTRQxPa5uv7hPypxUE6tE_RlGOluh-c&m=bqLD53fRz-777VQg0f3yqa5otQ3Up_pgY09fXeVM_yc&s=caIfT48JpSEd70IGE_bd4Ic0AlmCGnFAY6Nw_okBnYg&e=.

christos-c commented 8 years ago

Hi @yimingjiang, just to add to Dan's point, we should have two kinds of tests: a) functional; these will ensure that the programmatic components work given a known input (just like your current test) and b) machine-learning; these will ensure that the ML aspect performs as expected -- that is, produces correct predictions given a known input (that's what the CS446 datasets will ensure).

You can add the TestReal class in this PR. Just push to the add-adagrad-yiming branch and all the changes will show up here.

yj14n9xyz commented 8 years ago

Hi @christos-c,

I have a few questions:

  1. As Professor Roth @danr-ccg pointed out, it is better to have both hinge loss and LMS as loss function in AdaGrad. However, in AdaGrad`` class, is it better to add one more configuration field in constructor to specify which loss function to use, or create anAdaGradclass with abstractlearnfunction and make two inherited classes, namelyAdaGradHingeandAdaGradLMS``` implementing different actual update rule?
  2. I am not sure about "use the new algorithm also with the Averaged case". What does "averaged case" mean?
  3. @danyaljj Hi Daniel, for CS 446 HW 3 Online Learning, (http://l2r.cs.illinois.edu/~danr/Teaching/CS446-15/Hw/HW-hw3/2015/hw3.pdf) is there a solution data set being used? (since the data set is randomly generated from MATLAB script) And is a solution more detailed than the report (the report only provided a plot)?

@danr-ccg Hi Professor Roth, once cs 446 data set is fixed, I will run through all existing algorithms in LBJava and create a table of the results, as comparison. I will bring updates soon.

Thank you all!

danyaljj commented 8 years ago

Yiming,

  1. The former sounds better: "adding one more configuration field in constructor to specify which loss function to use"
  2. See the next one.
  3. Here are a bunch of solutions (uploading now...) https://www.dropbox.com/sh/652aayjbae8kjk0/AADQ8RzWSZKFTaFvRk3VVgZGa?dl=0

Since the dataset is generated randomly, there might be a little randomness involved; but average the results in these solutions should give a high-confidence answer to the expected performance. (I think this is what Dan means by "Average case" experiment).

danyaljj commented 8 years ago

One more comment: you can see that a module inside lbjava project contains examples. Would be great if you add another example using adagrad (or change one of the existing ones to use adagrad).

yj14n9xyz commented 8 years ago

@danyaljj Hi Daniel, thanks for the pointers. I just moved back to campus. Will get these done in a few days.

For "average case", does it mean that I run the same algo on 3 data sets generated using the same parameters and take the average on these 3 results?

The other question is, the data generated from CS 446 HW is very large, about ~50MB per file. Is it advisable to add lots of these files into LBJava? If not, how to run algo test on adagrad?

Thanks!

danr-ccg commented 8 years ago

Hi Yiming,

We can think again about these two issues that you bring up; first, maybe we don’t need to generate all the data. Take a more careful look at it, and let’s see if we really need all of it. Also, perhaps we can get rid of the data generation, generate it one, and record what expect to get from all the algorithms we have in LBJava.

        Dan

From: Yiming Jiang [mailto:notifications@github.com] Sent: Sunday, January 17, 2016 1:11 PM To: IllinoisCogComp/lbjava Cc: Roth, Dan Subject: Re: [lbjava] WIP: added adagrad class and unit test (#13)

@danyaljjhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_danyaljj&d=BQMCaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=EoEIPOcXriNKNTRQxPa5uv7hPypxUE6tE_RlGOluh-c&m=1D2mTsv7k2Yox_z_A6NLJPs1eAoCLnOUPJigwD16FiM&s=g1wP3NPBtF66IQU1Q1o2S77MB32Cdvie1brJGU38RGg&e= Hi Daniel, thanks for the pointers. I just moved back to campus. Will get these done in a few days.

For "average case", does it mean that I run the same algo on 3 data sets generated using the same parameters and take the average on these 3 results?

The other question is, the data generated from CS 446 HW is very large, about ~50MB per file. Is it advisable to add lots of these files into LBJava? If not, how to run algo test on adagrad?

Thanks!

— Reply to this email directly or view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_IllinoisCogComp_lbjava_pull_13-23issuecomment-2D172367652&d=BQMCaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=EoEIPOcXriNKNTRQxPa5uv7hPypxUE6tE_RlGOluh-c&m=1D2mTsv7k2Yox_z_A6NLJPs1eAoCLnOUPJigwD16FiM&s=Pg2vOACKxrmbJO87j34zzdn1HFfl2bM3dRM5hOdaLRM&e=.

yj14n9xyz commented 8 years ago

@danr-ccg

Hi Professor Roth,

Thanks for your suggested solutions.

There are three possible routes (we can do a combination of options, rather than just picking one):

1) Use a much smaller subset of CS 446 HW data. We keep a copy of the data set in LBJava and run all algorithms on this data set.

However, the drawback is that the data set is too small. It's no different than the data set that I am using right now. It may not reflect advantages of some algorithms. The good side is that it's small and simple. We can directly add into LBJava.

2) Re-implement data generation code in Java. Generate all data sets before running all algorithms and remove them when finished.

The good side is that it's minimal size. We are not storing any file in the repository. The pitfall is that it's going to take a much longer time to generate the data before running the tests.

However, from my understanding, these algo tests are not going to be run very often. We can record all the results from different algorithms and run the tests if needed.

3) Keep one full data set from CS 446 HW (about 50MB) and run all algorithms on it. For average case, I can simply provide the code and the recorded results, without storing any large files, except for one.

@christos-c @danyaljj Do you have any insights on the algo tests?

Thank you all !