You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hivemall.apache.org by takuti <gi...@git.apache.org> on 2017/06/20 22:29:28 UTC
[GitHub] incubator-hivemall pull request #87: [HIVEMALL-108] Support `-iter` option i...
GitHub user takuti opened a pull request:
https://github.com/apache/incubator-hivemall/pull/87
[HIVEMALL-108] Support `-iter` option in generic predictors
## What changes were proposed in this pull request?
Support `-iter` option in both **generic classifier** and **regression** by buffering input samples, with an auxiliary `-tol` option.
Ref. #79
## What type of PR is it?
Improvement
## What is the Jira issue?
https://issues.apache.org/jira/browse/HIVEMALL-108
## How was this patch tested?
- Unit test
- Passed with the same test cases which were used for the previous "manual" iteration using for-loops
- Manual test on EMR
- Same as what I did in #79
## How to use this feature?
Nothing has been changed except for the new options :)
---
### Notice
In order to check convergence, this PR adds `-tol` option; it is not `-eps` as pLSA #71 and LDA #66 have, because `-eps` already exists in generic predictors as an option for adagrad/adadelta. For the sake of unifying interface, `train_lda` and `train_plsa` might need to support `-tol` as alias of the existing `-eps` option.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/takuti/incubator-hivemall general-learner-iter
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hivemall/pull/87.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #87
----
commit 8fe56eee4856c0b3d791380ab81a47d89a4d6407
Author: Takuya Kitazawa <k....@gmail.com>
Date: 2017-06-19T22:38:15Z
Update loss function descriptions
commit 9b009051a9b817688e74ab81317eff44aaf152d0
Author: Takuya Kitazawa <k....@gmail.com>
Date: 2017-06-20T21:37:57Z
Support `-iter` and `-tol` option in generic predictors
commit b0011380db97081f35378c1ea5a973858d52940d
Author: Takuya Kitazawa <k....@gmail.com>
Date: 2017-06-20T21:50:50Z
Reduce buffer size
commit b2f0460f3c210c9afbb9cdf2c1247ec34e8c5db2
Author: Takuya Kitazawa <k....@gmail.com>
Date: 2017-06-20T22:08:39Z
Format
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
Thanks! will review in this week.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
[](https://coveralls.io/builds/12272359)
Coverage increased (+0.9%) to 40.884% when pulling **1f97bce04923977d55ef908eed0a6c2c92c44f05 on takuti:general-learner-iter** into **c06378a81723e3998f90c08ec7444ead5b6f2263 on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall pull request #87: [HIVEMALL-108] Support `-iter` option i...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on a diff in the pull request:
https://github.com/apache/incubator-hivemall/pull/87#discussion_r126605795
--- Diff: core/src/main/java/hivemall/GeneralLearnerBaseUDTF.java ---
@@ -220,6 +235,23 @@ protected PrimitiveObjectInspector processFeaturesOI(@Nonnull ObjectInspector ar
this.featureListOI = (ListObjectInspector) arg;
ObjectInspector featureRawOI = featureListOI.getListElementObjectInspector();
HiveUtils.validateFeatureOI(featureRawOI);
+ if (featureRawOI instanceof JavaStringObjectInspector) {
--- End diff --
We need to consider other possibilities for StringObjectInspector.
https://gyazo.com/4c7c3efb22f441db09f614457235b5c0
I'll handle this issue.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by takuti <gi...@git.apache.org>.
Github user takuti commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
@myui That's why I repeatedly tried a wide variety of combinations and finally found `-lambda 1e-6 -tol 1e-3f -iter 512` leads successful.
What `-tol` did is to compare the absolute difference between the last and current cumulative loss; that is, the initial implementation checked convergence based on **absolute difference**. Meanwhile, `-cv_rate` looks **relative difference**. Hence, in a precise sense, your commit feb9bb132afb8d81357f43cf2ee2d94f6e0aa9b6 is not "refactor," and I need to find a completely different optimal combination of hyper-parameters for testing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by takuti <gi...@git.apache.org>.
Github user takuti commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
@myui good catch & nice work~
If I remember correctly, I previously noticed the difference between `Text` and `String` as label of `FeatureValue`, and thus I replaced `NIOUtils.getString(src)` with `new Text(NIOUtils.getString(src))`. Since the unit test has been passed at the moment, I misguidedly considered the problem has been fixed.
I will try to look into the problem more.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
> What -tol did is to compare the absolute difference between the last and current cumulative loss; that is, the initial implementation checked convergence based on absolute difference. Meanwhile, -cv_rate looks relative difference. Hence, in a precise sense, your commit feb9bb1 is not "refactor," and I need to find a completely different optimal combination of hyper-parameters for testing.
`absolute difference => relative difference` DOES NOT explain too large loss in the middle of iterations. Even without conversion checks, it was never converged.
Iterations were converged when using absolute difference and `Assert.assertTrue(cumLoss / samplesList.size() < 0.5d);` was passing. Why?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by takuti <gi...@git.apache.org>.
Github user takuti commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
@myui Thank you for review and suggestion. I will work on them in a week.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
@takuti Thanks! will review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall pull request #87: [HIVEMALL-108] Support `-iter` option i...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on a diff in the pull request:
https://github.com/apache/incubator-hivemall/pull/87#discussion_r124191703
--- Diff: core/src/main/java/hivemall/model/FeatureValue.java ---
@@ -74,6 +83,16 @@ public void setValue(double value) {
this.value = value;
}
+ public void writeTo(@Nonnull final ByteBuffer dst) {
+ NIOUtils.putString(getFeatureAsString(), dst);
+ dst.putDouble(value);
+ }
+
+ public void readFrom(@Nonnull final ByteBuffer src) {
+ this.feature = new Text(NIOUtils.getString(src));
--- End diff --
No need to wrap `feature` in Text.
`this.feature = NIOUtils.getString(src);`
Text holds every strings in UTF-8.
UTF8 (1-4 bytes) => UTF-16 (2 bytes) `getChar` => UTF-8 will cause invalid character conversions.
Unfortunately, `hadoop.io.Text` cannot set internal bytes without conversion. `hivemall.utils.hadoop.Text2` do it using reflection though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
[](https://coveralls.io/builds/12272233)
Coverage increased (+0.6%) to 40.58% when pulling **1f97bce04923977d55ef908eed0a6c2c92c44f05 on takuti:general-learner-iter** into **c06378a81723e3998f90c08ec7444ead5b6f2263 on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
There are several design options to consider:
1) move `-iter` and `-mini-batch` functionalities to LeanerBaseUDTF
=> but, it cannot be used for MulticlassOnlineClassifier.. :-(
2) make BinaryOnlineClassfierUDTF/RegressionBaseUDTF the subclass of GeneraLearnerBaseUDTF
=> regression bug may occur ..
3) make Abstract IterativeLearnerBaseUDTF as the subclass of LearnerBaseUDTF
=> too complex object hierarchy (not good design)
4) implement `-iter` and `-mini-batch` functionalities in BinaryOnlineClassfierUDTF/RegressionBaseUDTF
=> duplicate code, hard for maintenance
5) Mixin iteration supoprt
=> callback is needed and not a pattern for mixin
6) generalize `-iter` support for MulticlassOnelineClassifierUDTF
=> not straightforward design
Also, `-mini_batch` design should be well revised for online perceptron variants.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
I think `-tol` is hard to configure by average users.
It can be replaced with `ConversionState`. It doubly checks that an iteration really converges.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
[](https://:/builds/12058918)
Coverage increased (+0.4%) to 40.407% when pulling **b2f0460f3c210c9afbb9cdf2c1247ec34e8c5db2 on takuti:general-learner-iter** into **2915a78c7acabbf0c26a9037fba305407539affa on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall pull request #87: [HIVEMALL-108] Support `-iter` option i...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on a diff in the pull request:
https://github.com/apache/incubator-hivemall/pull/87#discussion_r124519932
--- Diff: core/src/main/java/hivemall/GeneralLearnerBaseUDTF.java ---
@@ -318,66 +412,240 @@ protected void onlineUpdate(@Nonnull final FeatureValue[] features, final float
@Override
public final void close() throws HiveException {
super.close();
- if (model != null) {
- if (accumulated != null) { // Update model with accumulated delta
- batchUpdate();
- this.accumulated = null;
- }
- int numForwarded = 0;
- if (useCovariance()) {
- final WeightValueWithCovar probe = new WeightValueWithCovar();
- final Object[] forwardMapObj = new Object[3];
- final FloatWritable fv = new FloatWritable();
- final FloatWritable cov = new FloatWritable();
- final IMapIterator<Object, IWeightValue> itor = model.entries();
- while (itor.next() != -1) {
- itor.getValue(probe);
- if (!probe.isTouched()) {
- continue; // skip outputting untouched weights
+ finalizeTraining();
+ forwardModel();
+ this.accumulated = null;
+ this.model = null;
+ }
+
+ @VisibleForTesting
+ public void finalizeTraining() throws HiveException {
+ if (count == 0L) {
+ this.model = null;
+ return;
+ }
+ if (is_mini_batch) { // Update model with accumulated delta
+ batchUpdate();
+ }
+ if (iterations > 1) {
+ runIterativeTraining(iterations);
+ }
+ }
+
+ protected final void runIterativeTraining(@Nonnegative final int iterations)
+ throws HiveException {
+ final ByteBuffer buf = this.inputBuf;
+ final NioStatefullSegment dst = this.fileIO;
+ assert (buf != null);
+ assert (dst != null);
+ final long numTrainingExamples = count;
+
+ final Reporter reporter = getReporter();
+ final Counters.Counter iterCounter = (reporter == null) ? null : reporter.getCounter(
+ "hivemall.GeneralLearnerBase$Counter", "iteration");
+
+ try {
+ if (dst.getPosition() == 0L) {// run iterations w/o temporary file
+ if (buf.position() == 0) {
+ return; // no training example
+ }
+ buf.flip();
+
+ int iter = 2;
+ double cumLossPrev;
+ for (; iter <= iterations; iter++) {
+ cumLossPrev = cumLoss;
+ this.cumLoss = 0.d;
+
+ reportProgress(reporter);
+ setCounterValue(iterCounter, iter);
+
+ while (buf.remaining() > 0) {
+ int recordBytes = buf.getInt();
+ assert (recordBytes > 0) : recordBytes;
+ int featureVectorLength = buf.getInt();
+ final FeatureValue[] featureVector = new FeatureValue[featureVectorLength];
+ for (int j = 0; j < featureVectorLength; j++) {
+ featureVector[j] = new FeatureValue(buf);
+ }
+ float target = buf.getFloat();
+ train(featureVector, target);
+ }
+ buf.rewind();
+
+ if (is_mini_batch) { // Update model with accumulated delta
+ batchUpdate();
+ }
+
+ logger.info("[iter " + iter + "] cumulative loss: " + cumLoss);
+
+ if (Math.abs(cumLossPrev - cumLoss) < tol) {
--- End diff --
`cumLoss` is always 0 due to ` this.cumLoss = 0.d;`. So, this conversion check is not working properly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
https://issues.apache.org/jira/browse/HIVEMALL-135
Documentation about `-iter` option is lacking.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
[](https://:/builds/12109706)
Coverage increased (+0.4%) to 40.435% when pulling **8de5710a28e985ddaa24eceab3542796a6c524a3 on takuti:general-learner-iter** into **c06378a81723e3998f90c08ec7444ead5b6f2263 on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
It takes so long to reflect this kind of changes by review comments. Find commit logs for my points for review.
https://github.com/myui/incubator-hivemall/commit/ac062f2f5f8e0eb9ad093d4b92f478d791fcbbf4 is bug fix of ConversionState usage (cumulative loss is not reset at the beginning of 2nd iteration).
https://github.com/myui/incubator-hivemall/commit/b999196913874361b3e9e12c65418675a3070c23 avoided `-tol` and fixed to use existing ConversionState.
https://github.com/myui/incubator-hivemall/commit/94e16366d54b6fa4e58a7e78d9db16aea83eee3f revised variable scope in ConversionState.
https://github.com/myui/incubator-hivemall/commit/94e16366d54b6fa4e58a7e78d9db16aea83eee3f Fixed a bug in readFrom.
https://github.com/myui/incubator-hivemall/commit/65233ff69d653df432d88eb4ecbc9031a6de60c5 numSteps should be long to avoid overflow.
I guess `-tol` used in your PR was too small to find existing bugs in optimizer (not bugs in this PR).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall pull request #87: [HIVEMALL-108] Support `-iter` option i...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-hivemall/pull/87
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by takuti <gi...@git.apache.org>.
Github user takuti commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
@myui Do you mean I need to replace `FeatureValue[]` with `Feature[]` and support a new option `-int_feature`, in a similar way to what `FactorizationMachineUDTF` does?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
> Bug in 0c0e4ea caused b2f0460 to pass the test (?).
@takuti This assumption was correct.
`GeneralRegressionUDTFTest` is *illegally* passing when reverting fix in https://github.com/apache/incubator-hivemall/commit/0c0e4ea68187fa4b672a892774caaf74d5a5e058.
Fails on `GeneralClassifierUDTFTest` is due to relaxing conversion checks.
- `this.feature = new Text(NIOUtils.getString(src));` and `-cvRate 0.005`
Both `GeneralClassifierUDTFTest` and `GeneralRegressionUDTFTest` fails.
- `this.feature = new Text(NIOUtils.getString(src));` and `-cvRate 0.001`
Both `GeneralClassifierUDTFTest` and `GeneralRegressionUDTFTest` succeeds.
- `this.feature = NIOUtils.getString(src);` and `-cvRate 0.001`
`GeneralClassifierUDTFTest` succeeds but `GeneralRegressionUDTFTest` fails.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
I'm wondering to generalize `-iterations` and `-mini_batch` logic of GeneralLeanerBaseUDTF for BinaryOnlineClassifierUDTF and RegressionBaseUDTF as well.
https://gyazo.com/4ceaeb003ccad8587ca55e080ba6ccfa
It's not straightforward though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
[](https://coveralls.io/builds/12273467)
Coverage increased (+0.6%) to 40.58% when pulling **379a30ac81ca1801f8e9801da5865a39e28e2f9e on takuti:general-learner-iter** into **c06378a81723e3998f90c08ec7444ead5b6f2263 on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
[](https://:/builds/12094302)
Coverage increased (+0.1%) to 40.112% when pulling **1300b8c5bc12b77dd0b951e87841fccf808c1c30 on takuti:general-learner-iter** into **2915a78c7acabbf0c26a9037fba305407539affa on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
> Too large losses are probably observed as a consequence of inappropriate hyper-parameters. In the last implementation, I tried to test all loss functions under the same -eta0 and -lambda as:
@takuti Why test was succeeding at https://github.com/apache/incubator-hivemall/pull/87/commits/b2f0460f3c210c9afbb9cdf2c1247ec34e8c5db2 ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
[](https://coveralls.io/builds/12227721)
Coverage increased (+0.7%) to 40.699% when pulling **95927f64104bf75d071c8665d64afb48837fcb02 on takuti:general-learner-iter** into **c06378a81723e3998f90c08ec7444ead5b6f2263 on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
Applied some refactoring in https://github.com/myui/incubator-hivemall/commit/71b7c09fb2e603f91fb98753d9362d27128d66f5
It may exists bugs in the Optimizer implementation. Too large losses are found when doing early termination.
https://gyazo.com/c6f4155a465ecb3a7ea916a2ee68db85
https://gyazo.com/26b9c8d7ac1be42df8397df74c11d0e5
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by takuti <gi...@git.apache.org>.
Github user takuti commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
> It may exists bugs in the Optimizer implementation. Too large losses are found when doing early termination.
Too large losses are probably observed as a consequence of inappropriate hyper-parameters. In the last implementation, I tried to test all loss functions under the same `-eta0` and `-lambda` as:
```java
for (String opt : optimizers) {
for (String reg : regularizations) {
if (reg == "RDA" && opt != "AdaGrad") {
continue;
}
for (String loss : lossFunctions) {
String options = "-opt " + opt + " -reg " + reg + " -loss " + loss
+ " -lambda 1e-6 -cv_rate 0.005 -iter 512";
```
However, since each loss function draws different curve, optimal `-eta0` and `-lambda` should be different.
Finding optimal hyper-parameters for each combination of `-opt` + `-reg` + `-loss` is really hard, so I've updated `assert` condition; a new version of `GeneralRegressionUDTFTest` simply checks if iterative training improves MAE to some degree, by comparing with MAE w/ initial state of model.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by takuti <gi...@git.apache.org>.
Github user takuti commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
@myui Oh okay, will catch-up soon.
Refactoring and modification on ongoing PR are personally confusing by the way...let us first discuss here, and use GitHub's "review" feature more...it's an OSS project!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by takuti <gi...@git.apache.org>.
Github user takuti commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
> generalize -iterations and -mini_batch logic of GeneralLeanerBaseUDTF for BinaryOnlineClassifierUDTF and RegressionBaseUDTF as well
I considered a little, but, as you suggested many possible ways, this complex topic should be handled more carefully as a different issue. The first step will be to support `-iter` in each of `BinaryOnlineClassifierUDTF` and `RegressionBaseUDTF`.
If you are okay to treat them as a different issue, I am ready for review & merge this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by takuti <gi...@git.apache.org>.
Github user takuti commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
@myui If I understand correctly, the feature-type-related issue has been fixed 3c753bbdaaf2c145048fe885bb44cb62f7a08232
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
[](https://:/builds/12108721)
Coverage increased (+0.1%) to 40.14% when pulling **26bdfab6dfe63f66881bf1de0a9bca7eb612002d on takuti:general-learner-iter** into **c06378a81723e3998f90c08ec7444ead5b6f2263 on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
Bug in https://github.com/apache/incubator-hivemall/pull/87/commits/0c0e4ea68187fa4b672a892774caaf74d5a5e058 caused https://github.com/apache/incubator-hivemall/commit/b2f0460f3c210c9afbb9cdf2c1247ec34e8c5db2 to pass the test (?).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
It might exist more complex potential bug in the iteration support.
In the first iteration, feature index can be `Text` but `String` in the following iterations.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
@takuti
- store feature type in GeneralLearnerBaseUDTF
https://github.com/takuti/incubator-hivemall/blob/95927f64104bf75d071c8665d64afb48837fcb02/core/src/main/java/hivemall/GeneralLearnerBaseUDTF.java#L222
- wrap feature writes to use type info
https://github.com/takuti/incubator-hivemall/blob/95927f64104bf75d071c8665d64afb48837fcb02/core/src/main/java/hivemall/GeneralLearnerBaseUDTF.java#L305
- wrap feature reads to use type info
https://github.com/takuti/incubator-hivemall/blob/95927f64104bf75d071c8665d64afb48837fcb02/core/src/main/java/hivemall/GeneralLearnerBaseUDTF.java#L483
Refer https://github.com/apache/incubator-hivemall/blob/master/core/src/main/java/hivemall/fm/FactorizationMachineUDTF.java#L665 and https://github.com/apache/incubator-hivemall/blob/master/core/src/main/java/hivemall/fm/FactorizationMachineUDTF.java#L630 for example
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hivemall issue #87: [HIVEMALL-108] Support `-iter` option in gener...
Posted by myui <gi...@git.apache.org>.
Github user myui commented on the issue:
https://github.com/apache/incubator-hivemall/pull/87
@takuti No.
`enum FeatureType { JavaString, Text, JavaInt, WritableInt, WritableLong }`
• `this.featureType = resolveType();` in `initialize()`.
• wrap feature read/write using `featureType`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---