[GitHub] incubator-joshua issue #79: JOSHUA-324 renamed LICENSE, removed minification...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/79 travis fails because of a much earlier error where the download-deps.sh script causes things to hang indefinitely. Fixing that first, then will re-test this. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #79: JOSHUA-324 renamed LICENSE, removed minification...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/79 Thanks for catching this, Kellen. I'll pull this in as soon as travis completes. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #69: JOSHUA-290 - provide Joshua as a bundle
Github user mjpost commented on a diff in the pull request: https://github.com/apache/incubator-joshua/pull/69#discussion_r82191791 --- Diff: src/main/java/org/apache/joshua/decoder/package-info.java --- @@ -23,4 +23,7 @@ * of any actual decoding algorithm. Rather, such code is in * child packages of this package. */ -package org.apache.joshua.decoder; \ No newline at end of file +@Version("0.1.0") +package org.apache.joshua.decoder; + +import org.osgi.annotation.versioning.Version; --- End diff -- Yes, 7 maven-multi-module is merged into 7, so that's the best place for it. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #70: validate the original implementation of SARI
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/70 Hi @cocoxu â There don't appear to be any changes in this commit, just the changing of two blank lines. I assume that means that Joshua already has everything that matters? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #65: This PR is a first attempt to minimize ngram arr...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/65 I did some timing tests on commit 3ba83f84e8258a784fcef509fc9b158d44f15c66, and didn't see any change. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #66: Merge pull request #1 from apache/7_confs...
Github user mjpost closed the pull request at: https://github.com/apache/incubator-joshua/pull/66 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #66: Merge pull request #1 from apache/7_confs...
GitHub user mjpost opened a pull request: https://github.com/apache/incubator-joshua/pull/66 Merge pull request #1 from apache/7_confsystem You can merge this pull request into a Git repository by running: $ git pull https://github.com/fhieber/incubator-joshua 7_confsystem Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-joshua/pull/66.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 #66 commit 7726e7d4248556814e41d8dc4feba1f29d5c771c Author: Felix Hieber Date: 2016-09-17T17:06:01Z Merge pull request #1 from apache/7_confsystem --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #58: Moved new unit tests (former regression tests) t...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/58 Ah, right. Note that the test works from the command line: cd $JOSHUA/joshua-core cat src/test/resources/bn-en/hiero/input.bn | $JOSHUA/bin/joshua-decoder -c src/test/resources/bn-en/hiero/joshua.config > output diff output src/test/resources/bn-en/hiero/output.gold --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #58: Moved new unit tests (former regression tests) t...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/58 My guess is that grammar weights need reversing, but then I wonder why we haven't had to do that for other tests. I'll check this out later in the morning. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #52: Moved regression tests bn-en/hiero to unit test
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/52 Okay, on second thought, as we discussed, lets do these on master. Merging wasn't that difficult. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #54: Moved regression tests bn-en/packed and bn-en/sa...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/54 Progress on JOSHUA-299. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #52: Moved regression tests bn-en/hiero to unit test
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/52 Okay, this is merged in, obviously. But merging these back into 7 is turning into a pain. Since development is stopping on master, lets just do the rest of these on the 7 branch directly, okay? @michael-aloys @KellenSunderland --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #24: Maven multi-module project layout proposal
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/24 No big deal, but if you update this pull request to be against the "7" branch instead of master, it will be marked as resolved. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #49: 7 with master
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/49 Pushed! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/42 Yes, please merge master into 7! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/42 So am I correct that this is ready to merge into master? And @maxthomas, you think you can merge this into 7? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #47: Fix Travis-CI by adding a step to install ant on...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/47 A better option here would be to upgrade the [Berkeley Aligner](https://github.com/joshua-decoder/berkeleyaligner/tree/master/src/edu/berkeley/nlp) (Joshua's version) to Maven, and distribute it as a maven resource. This reflects the fact that the Berkeley Aligner code â already almost a decade old â is unlikely to change much. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #48: Fixed crashing when using Trie based KenLM model...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/48 Holy smokes, thanks for tracking this down. So if I understand correctly, this only occurs under the following circumstances: - decoding with multiple KenLM language models - built with different vocabularies (the usual case) - a hash collision occurs and returns a state containing an ID that is invalid in the calling KenLM Do you have any idea how often hash collisions actually occur? I wonder if turning off sharing of KenLM states across LMs would also have worked, with little to no effect on performance. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/42 So, @maxthomas â what do you think about either of the following: - You (a) remerge master into this, and then (b) merge master into 7 - Skip (a) and just rework this against the 7 branch? I think this is good to have in the codebase, but I'm not sure that it's critical to have for the 6.1 release. But if it's not too much work to do both, I'm not against trying. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #45: Refactored the threading code in the JoshuaDecod...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/45 For posterity, here's a diagram of the complexity that's being simplified. [20160829105424778.pdf](https://github.com/apache/incubator-joshua/files/442923/20160829105424778.pdf) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/d4fdbfd88bab99e244d3ed1fc9cff4ba5e6d124c#commitcomment-18760447 I think maybe no, since all the feature functions are loaded using reflection. This could be a method available only on StatefulFF. Or maybe there is a better way to do it. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/d4fdbfd88bab99e244d3ed1fc9cff4ba5e6d124c#commitcomment-18742703 The whole static global feature index is a mess. You've gotten rid of the dense feature indices on the 7 branch. What if we get rid of the global state index as well? We could fix this in initializeFeatureFunctions, with the following test: ... this.featureFunctions.add(feature); int stateIndex = 0; if (feature instanceof StatefulFF) { feature.setStateIndex(stateIndex); stateIndex++; } Thoughts? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/ff410c297a149400db3cb553b11a930ad01dc7ed#commitcomment-18737267 Thanks, Lewis. What's your comment line-wrap set to? We've been using 100 and it would be nice to be consistent. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #44: Moved test case files
GitHub user mjpost opened a pull request: https://github.com/apache/incubator-joshua/pull/44 Moved test case files Moved test cases files from resources/ to src/test/resources. Also fixed bug in LmOovFeatureTest. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/incubator-joshua JOSHUA-299 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-joshua/pull/44.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 #44 commit 20e6bf4b069d261718f819a72c9e9b2c62bcbb26 Author: Matt Post Date: 2016-08-22T22:03:53Z Moved test file locations from resources/ to src/test/resources commit 4812fedee89d3a6dac4e8333a1d62242f3b5ac9c Author: Matt Post Date: 2016-08-23T02:36:09Z missed one commit d4fdbfd88bab99e244d3ed1fc9cff4ba5e6d124c Author: Matt Post Date: 2016-08-23T02:57:06Z bugfix: resetting global decoder state We are getting random failures in LmOovFeatureTest.java, but only when run as a group. I printed the stack trace around the error and noticed it's a call to getting the state index in ComputeNodeResult, with a state index of 1, which shouldn't happen because only one LM is loaded. So it seems that the bug is cause by some earlier test not calling Decoder.resetGlobalState() in cleanup. So I put a call to that in the constructor. This passes the tests but I'm not sure it's correct, and it's definitely not the right way to go about things. The ideal way to solve this is to get rid of global state. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #43: Phrase-based decoder rewrite
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/43 Followup: this appears to have something to do with a failed global state cleanup from a prior test run. Fixed (temporary at least) at commit d4fdbfd88bab99e244d3ed1fc9cff4ba5e6d124c. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #43: Phrase-based decoder rewrite
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/43 Okay, I am merging this, since it's just as random as before. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #43: Phrase-based decoder rewrite
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/43 More information: - On OS X, remove $JOSHUA/lib/libken.dylib, run "mvn test" â FAILURE - Same, but run the individual test from Eclipse â PASSES - Same, but run all tests from Eclipse â PASSES --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #43: Phrase-based decoder rewrite
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/43 Strange: the LmOovFeatureTest is failing again when KenLM is not available, but now it's on a Mac OS X instance, and not on the Linux one. I'm tempted to disable this test because its failure is so random; on the other hand, maybe the random failure is indicative of a deeper problem. Can anyone repeat this failure? I can repeat it on my laptop when I remove $JOSHUA/lib/libken.dylib. @KellenSunderland @fhieber --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #43: Phrase-based decoder rewrite
GitHub user mjpost opened a pull request: https://github.com/apache/incubator-joshua/pull/43 Phrase-based decoder rewrite The phrase-based decoder used to add nonterminals to every phrase-based rule, treating all such rules as left-branching ones. This was a hassle because everything had to be converted, e.g., after extracting from Thrax. Now, the phrase tables have no nonterminals on the source and target sides. Instead, glue rules are used. This means this is not backwards compatible. Phrase-based language packs will have to be recompiled, but this needs to be done anyway. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/incubator-joshua JOSHUA-284 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-joshua/pull/43.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 #43 commit dcc7e7ee72228de08b70003a49344c2614eaedbe Author: Matt Post Date: 2016-08-16T22:13:06Z large commit converting phrase-based decoding to new rule format Not working yet, but much of the code is redone and future estimates are being computed correctly commit 32504c47bbc90b3fd4a8d02298b9758fa8126a44 Author: Matt Post Date: 2016-08-16T22:13:50Z updated scripts to work with the new format commit 48a9aad7873b969230aad90d6e0c61e13ae2d2b4 Author: Matt Post Date: 2016-08-16T22:14:15Z repacked the grammar commit dac822d15145614c33f5fb12d2797e1f91825bb3 Author: Matt Post Date: 2016-08-17T10:23:57Z missed file in commit commit b1ec62711d15f3b692b6a7026752123f75522f6e Author: Matt Post Date: 2016-08-17T10:24:07Z enabled test commit 1022699cc744fa9fbc21f4b19122f51e3985a371 Author: Matt Post Date: 2016-08-17T10:24:46Z temporary commenting-out of very verbose output commit 2e746c1864ca7eb6be27f2fca3ab258c9ebe7adb Author: Matt Post Date: 2016-08-19T18:14:18Z changed order of assert() args commit 048b2e30f849de3f1ac82e6017ea2aab299f6b8d Author: Matt Post Date: 2016-08-19T18:15:18Z removed RHS nonterminal commit af4ef88d5a6a6a1cc4167ec421b4b6bd1a91dc0a Author: Matt Post Date: 2016-08-19T18:15:36Z added derived directories commit 9b73d6147a61580058cc57c86c1f623f44b7452a Author: Matt Post Date: 2016-08-19T18:16:47Z build two nodes over terminal productions commit 5719c8cff728499bffd1053462351340f1d91353 Author: Matt Post Date: 2016-08-19T18:17:21Z fixed distortion computation to work with new format code now produces a translation on my test case, though it's not the correct one commit eb00223870c7683cf8e557ab689a1979fb36ec1d Author: Matt Post Date: 2016-08-20T00:43:58Z converted from span -> separate i, j commit 473b3016562677671f70a19cd15d67a2bc1a5c83 Author: Matt Post Date: 2016-08-20T00:44:14Z off-by-one error in computing future estimates commit 574cb36b5e1b610e37eda81d6d76b4318c141a4c Author: Matt Post Date: 2016-08-20T00:44:44Z bugfix: this is (probably) supposed to return the pruning estimate commit 16d5647bee30345ffa56b5b7d5bebc1021afa3fa Author: Matt Post Date: 2016-08-20T00:45:12Z fixed computation of distortion commit 36cde50ba37df9c9b2ead6b063ac5935e3dd253d Author: Matt Post Date: 2016-08-20T13:30:42Z moved comparator into Candidate commit 49dbf8cbaf2f1e0c648f8eb705ab3887aa06b039 Author: Matt Post Date: 2016-08-20T13:31:18Z removed nonterminals from OOV rules commit e3b60ca9a7fea7d25a8533b630a1a66d29349a6f Author: Matt Post Date: 2016-08-21T11:53:26Z minor cleanup of assignment logic commit 293db94c2853f7dc15bd6fecdf3b39bd3a4b4965 Author: Matt Post Date: 2016-08-21T11:53:41Z Bug fix in reporting inside cost â everything now works commit 0e49bc537b05549930802bf6c187b849c4c67adb Author: Matt Post Date: 2016-08-21T11:55:55Z added debug-joshua which sets debugging and uses classes instead of the jar This means you can run the command-line version while in Eclipse without having to rebuild the jar file (which is time-consuming). commit d6820c6f3bc41ca87dfff4a8ed18172de4f849e6 Author: Matt Post Date: 2016-08-21T12:01:17Z removed debugging output commit cd3ff0c6d0d2ad959cd4f292d9ee02f4e7da8b0a Author: Matt Post Date: 2016-08-21T12:01:38Z removed alignments from test, created new test with alignments (currently not working...) commit 25d28fe2ce32a4b130a4412e982d6e16d5af8afc Author: Matt Post Date: 2016-08-21T12:24:58Z small cleanup commit d28b4f39c578197803beba2c376db5ed95774576 Author: Matt Post Date: 2016-08-21T17:36:37Z Merge branch 'master' into JOSHUA-284 commit 12b834e271a361417cbdabf79036538493cdb122 Author: Matt Post Date: 2016-08-22T20:59:43Z Now building HGNode over the phrase when it's added This should
[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/42 All right; maybe hold off and let me finish and merge in one other large-ish commit, and then I'll take a look at this. It's possible it will be easier than I anticipate. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #42: Fix various issues related to resources, warning...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/42 This actually looks really great, but I'm worried about the complexity of merging this with the 7, which moves every file and changed a bunch with refactoring the feature function interface. How much work went into this, and how hard would it be to repeat it? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #41: JOSHUA-265 Major refactoring of features/rules/g...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/41 This looks great. I put it on a new branch off 7 so that we can Travis-CI it (on the idea that 7 is its own master). Once we get all the regression tests removed and merged in from master, I'll merge this back into 7. Tagging this on JOSHUA-265 and JOSHUA-273. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #40: JOSHUA-302 - Remove dependency on concurrent/con...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/40 I actually think this might have been auto-inserted by Eclipse somehow (resolving to the wrong version of a library). So, good find, and thanks for submitting. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/3e2e053db9cafcdc5b4885a0f6fa5e54a63bc468#commitcomment-18672288 No --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #24: Maven multi-module project layout proposal
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/24 Awesome, that merged really cleanly. There is now a pushed 7 branch that we can begin work on for release 7. [Travis-CI](https://travis-ci.org/apache/incubator-joshua/builds/152931806) is testing this as I write. Once we release 6.1, I'll merge 7 into 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #24: Maven multi-module project layout proposal
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/24 Yeah, that was one of our main concerns, too. Which is why we want to get this into master as soon as possible. There are some nasty merges ahead of us, but the unit tests should help pull us through :) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #24: Maven multi-module project layout proposal
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/24 We wanted to include this in 6.1, but felt that a responsible manager would say that it is non-critical and that we should instead base our future release on it. We rebuilt this yesterday on top of master. Within the next few days, the new version should get pushed up. I will pull that into a "7" branch, which we'll start work on. Once 6.1 is officially released, we'll merge "7" back into master. Sound okay? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #24: Maven multi-module project layout proposal
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/24 Commenting here so this discussion gets tied to issue [JOSHUA-295](https://issues.apache.org/jira/browse/JOSHUA-295?jql=project%20%3D%20JOSHUA%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20key%20DESC%2C%20priority%20DESC). A re-implementation of this will serve as the base for Joshua 7, currently scheduled for spring release. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #37: JOSHUA-292 - Add travis CI badge to README.md
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/37 Thanks! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #36: Demo
Github user mjpost commented on a diff in the pull request: https://github.com/apache/incubator-joshua/pull/36#discussion_r73451334 --- Diff: src/main/java/org/apache/joshua/server/ServerThread.java --- @@ -225,15 +225,18 @@ private void handleMetadata(String meta, JSONMessage message) { String source = argTokens[1]; String target = argTokens[2]; String featureStr = ""; + String alignmentStr = ""; if (argTokens.length > 3) featureStr = argTokens[3]; - + if (argTokens.length > 4) +alignmentStr = " ||| " + argTokens[4]; --- End diff -- Phrase pairs can optionally have word level alignments: [X] ||| je mange ||| i eat ||| *scores* ||| 0-0 1-1 This creates word level alignments je/i and mange/eat. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/0663a9c7f704e1fc911d1792910ca9337840553b#commitcomment-18511694 @KellenSunderland do you have a minute to look at this? I thought this had passed CI tests, but Travis [is now telling me](https://travis-ci.org/apache/incubator-joshua/builds/148722346) that it didn't. It runs fine on my OS X machine and on a Linux machine I have access to, but seems to fail the generic test here. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #36: Demo
GitHub user mjpost opened a pull request: https://github.com/apache/incubator-joshua/pull/36 Demo Made some improvements to the Joshua demo under demo/. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mjpost/incubator-joshua demo Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-joshua/pull/36.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 #36 commit c465e71530eecead797f73c06da25f1c0cff28ed Author: Matt Post Date: 2016-08-02T13:08:42Z fiddling with demo commit 1038a144468b48408f3eca1122efb3c8f84e4537 Author: Matt Post Date: 2016-08-02T13:41:39Z quick add rules for OOVs, format fiddling commit aedeafd0df1a30ecf686193cd9446ba7e569e84f Author: Matt Post Date: 2016-08-02T16:14:22Z HTTP server now projects case commit 3387b16befb1e8121a05e7b3a13d041bdeb7260b Author: Matt Post Date: 2016-08-02T16:19:44Z fixed to work with detokenized output commit ca6fc49dc853ea07189e65b2df9e77f36bbfd7dd Author: Matt Post Date: 2016-08-02T16:20:03Z denormalizes output commit 84301b9f04b3ffeecf35432309b1ca762c692313 Author: Matt Post Date: 2016-08-02T16:20:10Z remove sentence marker commit 81b33d2218e51bc70f3ef42c2f0cd3c0d3059340 Author: Matt Post Date: 2016-08-02T16:38:27Z added alignment point in custom rules --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #33: Refactored unit tests to all use TestNG, removed...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/33 This passes for me, and $JOSHUA/lib is the currently correct place. I'm fine to move it if there is a more conventional location. Would $JOSHUA/target make more sense, or something else? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #33: Refactored unit tests to all use TestNG, removed...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/33 Does this pass your tests? When I run it, I get errors related to maven-surefire-plugin. I thought I'd check if you know what this is about before I dig into it. Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.19.1:test (default-test) on project joshua: Execution default-test of goal org.apache.maven.plugins:maven-surefire-plugin:2.19.1:test failed: The forked VM terminated without properly saying goodbye. VM crash or System.exit called? [ERROR] Command was /bin/sh -c cd /Users/post/code/joshua && /Library/Java/JavaVirtualMachines/jdk1.8.0_60.jdk/Contents/Home/jre/bin/java -Djava.library.path=/Users/post/code/joshua/lib -jar /Users/post/code/joshua/target/surefire/surefirebooter3138892612018380852.jar /Users/post/code/joshua/target/surefire/surefire2495440792813112582tmp /Users/post/code/joshua/target/surefire/surefire_03287663746024243525tmp --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #32: JOSHUA-286 - Replace old joshua-decoder.o...
Github user mjpost commented on a diff in the pull request: https://github.com/apache/incubator-joshua/pull/32#discussion_r72616102 --- Diff: scripts/support/make-release.sh --- @@ -40,7 +40,7 @@ echo "Bundling up joshua-$version" [[ ! -d release ]] && mkdir release rm -f joshua-$version && ln -s $JOSHUA joshua-$version -wget -r http://joshua-decoder.org/ +wget -r http://joshua.apache.org/6.0/ --- End diff -- This was intended to bundle a crawl of the website with the release. It should be removed. Joshua is now hosted on Confluence, so the directory paths now longer work. Also, Confluence has better tools for documentation export if we should decide to use them in the future. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #32: JOSHUA-286 - Replace old joshua-decoder.o...
Github user mjpost commented on a diff in the pull request: https://github.com/apache/incubator-joshua/pull/32#discussion_r72615877 --- Diff: .gitignore --- @@ -42,6 +42,7 @@ doxygen_*.tmp .cachepipe joshua-decoder.org +joshua.apache.org --- End diff -- Yeah, agreed. I used to bundle a crawl of the website with the repo, but am no longer going to do so. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #29: Added option to fire a dense language model oov ...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/29 Looks great to me, thanks for the contrib! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/3fb809c444c7855255b51080c368071bc508be1f#commitcomment-17995203 I don't have any experience that would drive this decision, but I agree we should decide on one. Currently, eclipse tells me there are 42 junit tests and 94 (87 passing / 2 failing / 5 skipped) testng tests. It also gives me a tool to convert junit tests to testng. @fhieber, does that change your mind? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/748cf32f956d3fd5772f62e023d0cc8d70a7dcad#commitcomment-17994955 No problem, it didn't get triggered until I had a small case where I actually asked for the alignments on output. I suppose we could also have fixed this by passing UKNOWN_OWNER_ID, we can switch it if you prefer. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/748cf32f956d3fd5772f62e023d0cc8d70a7dcad#commitcomment-17994847 Sure â UNKNOWN_OWNER is a String, so the constructor that gets called is [this one](https://github.com/apache/incubator-joshua/blob/master/src/main/java/org/apache/joshua/decoder/ff/tm/Rule.java#L157), which interprets the last argument as an alignment, not an owner. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #25: ClassLMs: fixed a bug with class-based lms not m...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/25 Okay, this checks out, producing virtually the same scores in two settings: (1) re-decoding a test set with an existing model (built with the non-state-minimizing LM), but swapping in the StateMinimizingLanguageModel, and (2) te-tuning with the StateMinimizingLanguageModel (allowing it to learn its own weight), with all other items the same. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #26: Owner Ids are maintained in separate mapping now
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/26 Great, this checks out for me. Since it contains the class LM fix, too, though, I'll wait to merge until those tests pass. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #25: ClassLMs: fixed a bug with class-based lms not m...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/25 This passes my regression tests. I am now testing it on an actual tuning / testing run for en-fi. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #24: Maven multi-module project layout proposal
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/24 Thanks, @logogin. I have only glanced at this, but it seems like a good idea. I hope to be able to take a look tonight, and expect all will be well. Do you want more discussion, or should I merge tonight if it seems okay to me? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/6d2213a20b74432fc7cb131c732f7507b74053e9#commitcomment-17933088 In src/main/java/org/apache/joshua/decoder/StructuredTranslation.java: In src/main/java/org/apache/joshua/decoder/StructuredTranslation.java on line 57: Hi Felix â I guess I probably don't. I put this in so as to have the convenience functions of querying the maps (that are in FeatureVector), and didn't realize why you guys had stuck with just a HashMap. I'll revert it. In general, this relates to JOSHUA-273: I want to remove all the output code from KBestExtractor and place it in StructuredTranslation (and then, in turn, to collapse Translation and StructuredTranslation). There are at least two ways to do this; do you have a preference? - StructuredTranslationFactory could be called to create the necessary entries in StructuredTranslation, which would then just be stored as strings. This approach makes StructuredTranslation basically just a container that is operated on. - StructuredTranslation can do its own computation, formatting the "outputFormat" line from the config file. This view makes StructuredTranslation itself a bit smarter. matt --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #24: Maven multi-module project layout proposal
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/24 This looks cool to me. Is this a recent pattern or something? I just noticed a similar approach on the [deeplearning4j](https://github.com/deeplearning4j/deeplearning4j) repo. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request #:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/3fb809c444c7855255b51080c368071bc508be1f#commitcomment-17813771 So we should choose either junit or testng and settle on that. Which should we use? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #22: This patch enables junit tests. Fixes a bug with...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/22 I am in fact, thanks, @logogin. I'll try to post next 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #22: This patch enables junit tests. Fixes a bug with...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/22 I am happy to have just learned about "mvn 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua issue #22: This patch enables junit tests. Fixes a bug with...
Github user mjpost commented on the issue: https://github.com/apache/incubator-joshua/pull/22 Okay, I merged this. Notes: - To get it working, I had to rerun "mvn eclipse:eclipse" - I also had to rebuild the JUnit test configuration. I think this might have to do with me not quite understanding JUnit eclipse integration - I still have one unit test failing: `givenPackedGrammar_whenNTranslationsCalledConcurrently_thenReturnNResults()` (in `MultithreadedTranslationTest`) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use Maven to b...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12 Actually @chrismattmann it's just a fast-forward at this point â all is merged on JOSHUA-252, it's just not showing up on this PR. https://github.com/apache/incubator-joshua/tree/JOSHUA-252 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use Maven to b...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12 Unless there are any objections, I'd like to merge this branch back into master. There are a few small issues, but none that can't be addressed with documentation. I'd love to do this either tonight or tomorrow, so please voice any objections as soon as possible. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use Maven to b...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12 https://github.com/joshua-decoder/berkeleylm/ --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-258 Add back penn-treebank-(de)tokeniz...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/8 @lewismc, do you mind closing this PR? I don't have write access and can't, and this has been fixed elsewhere. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: New Juju charms for deployment process & move...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/16 Okay, just pulled this into JOSHUA-252. It will get merged with master when we finish the Apache transition. Thanks, @buggtb! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use Maven to b...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12 So here is what is outstanding: - We need to either (a) get a new BerkeleyLM in Maven (anyone know how to do this?) or (b) pull the codebase into Joshua (is this permissible, including the renaming?) - I think we can safely leave Thrax as an external dependency that the user has to install, and provide instructions on the website for how to do so. - We can do the same for KenLM, and make the BerkeleyLM build and runtime tools the default. That way we don't have to mess with any shell scripts from Maven, which is not advised (because of portability concerns). matt --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use Maven to b...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12 Another issue: BerkeleyLM is currently pulled from the Maven repository, but it is very old ([1.1.2](http://mvnrepository.com/artifact/edu.berkeley.nlp/berkeleylm/1.1.2), but [1.1.6 is available](https://github.com/joshua-decoder/berkeleylm/commits/master)). I'm pretty sure its author has abandoned it; how can we get it updated? Joshua has a few bug fixes on it, even. One option: we could just roll the codebase into Joshua (it's Apache 2.0). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: Some changes to structured translation interf...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/20 Okay, I'll plan for that route. A technical note, though: from my understanding, rebasing is essentially changing the history, and doesn't work if you've pushed the history upstream. So it'll have to be a merge. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: Some changes to structured translation interf...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/20 Okay, good to know. I didn't get to this yesterday, and have some other things to do today, but provisionally, it sounds like it will be good to merge this into a new branch (off master), then to merge this into JOSHUA-273, and then merge all that back in (a) when it's complete and (b) once JOSHUA-252 is done. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: Some changes to structured translation interf...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/20 So it seems this isn't compatible with JOSHUA-273 (which is fine), but provides some of its functionality. One concern I have is the complexity of juggling the master branch, JOSHUA-252, JOSHUA-273, and these changes in my head right now. I'd really like to get JOSHUA-252 merged in to help simplify things. We would then want to merge either (a) JOSHUA-273, after having Decoder.decodeAll() return Translations, (b) merge this PR, or both. The reason I'm pushing a bit for JOSHUA-273 is because I also removed a lot of the redundancies in k-best extraction and so on, collapsed Translation and StructuredTranslation, and did some other things. It seems that it subsumes this PR? In which case, it might be less complex to go that route. But correct me if I'm wrong, or if JOSHUA-273 is headed in the wrong direction, or if this PR has some short-term advantage that can help us out in the meantime. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: Some changes to structured translation interf...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/20 I plan to look at this this morning. One question: how does this relate to the changes in JOSHUA-273? Are they more or less along the same lines, or do they do different things? If I changed JOSHUA-273 so that Decoder.decodeAll() returns a Translation object, would that accomplish what you've done here? If you have any notes on this (or even an I don't know) that will inform my 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: Added the licence header
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/19#issuecomment-52934 Thanks! You should have commit access soon. I think @lewismc knows how to do this? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-37297 Okay, great! That worked. I assume the edits to $JOSHUA/bin/joshua mean that eclipse compiled files will override the jar? So I can do fast development in Eclipse? We still have to solve the KenLM problem. My tests ran because I linked $JOSHUA/ext and $JOSHUA/lib to directories on master. Do we have ideas for how to pull in KenLM and compile it? And BerkeleyLM? I think that's the big thing remaining. After that, I'll have to go over the log messages in a bit more detail. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: FIX: override logger settings at ru...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/18#issuecomment-36910 Great, that solved the problem, thanks Thamme. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-04726 Yes, I hope I was clear that this is all great work. We just have to figure out how to get log4j to read the config file, which it's currently not doing. Then, we can make sure the test cases pass, and merge back into 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-3 Just to be clear, not sure it needs to be reverted. We just need to find a way to make sure the config file gets read and that logging gets set to INFO by default. And logging should all go to STDERR, I think, not STDOUT. In general, too, having INFO prepended to all informative lines is a bit ugly. Is this a useful convention for reasons I can't understand? It's often useful to see the decoder working with just these high-level labels, and I don't mind debug lines being prepended with DEBUG. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-222143162 Okay, can't stop scratching this itch. On even further investigation, with some System.err.println()s, I discovered that the properties file isn't getting loaded correctly. Consequently, isDebugEnabled() is always returning true, but the logging statements are not printed anywhere visible. So the work is getting triggered, and is just not seen, and that is the problem. I've pushed up the merge. Can you folks take care of making sure that the logger file is running correctly? You can test this by running cd $JOSHUA/src/test/resources/bn-en/hiero/ time ./test.sh Depending on your hardware, of course, that should run in just a few seconds tops. Incidentally, there are at least two other problems I am hoping you can help address? - I see how to build a big tarball and run Maven that way, but that's a pain for development, since you have to incur the time of rebuilding the tarball. Is there a way to just run against the files in target/classes directly? I've tried this, but the problem is that the jar dependencies don't seem to get downloaded anywhere. - Your response might be, "actually, you just have to regenerated the tarball, that's the proper way to do things." In which case, can you describe how to make eclipse do this? Right now, when I'm developing, eclipse is always compiling files, so you can just switch over and test right away. It's a small but significant paint to have to type "(cd $JOSHUA; mvn compile assembly:single install" - I really like being able to use the `-v` (verbosity) flag to Joshua, which sets the debugging level. This is much simpler than changing a properties file and linking to that from the command line. Is it possible to set the log4j debug level programmatically based on the value of -v? We could do -v 0 = OFF -v 1 = INFO -v 2 = DEBUG Or I would be okay to switch to those values directly (e.g., "-v OFF"). But the former seems more Unix-like. Thanks again for all your work on this! matt --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-222120221 I went into joshua/decoder/chart_parser/ComputeNodeResult; adding and removing the following lines makes a huge difference in speed. So it looks like logging is neither (a) time-safe nor (b) thread-safe. Or that something is wrong. I can't get the decoder to output any statements using the logging properties. It's sort of a pain to have to rebuild a tarball every time you want to test something... FYI, I won't have any more time to spend on this before Tuesday. ``` if (LOG.isDebugEnabled()) { LOG.debug("ComputeNodeResult():"); LOG.debug("-> RULE {}", rule); } [snip] if (LOG.isDebugEnabled()) { LOG.debug("-> item.bestedge: {}", item); LOG.debug("-> TAIL NODE {}", item); } ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-222118458 On a hunch, I went through and removed all the calls LOG.*() in Decoder, DecoderThread, JoshuaDecoder, Translation, Chart, DotChart, and ComputeNodeResult. This fixed the problem; the decoder now runs as fast as before. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-222032586 Okay, I just pushed up the changes after merging in master and making some other changes to that the tests will run (with manual calls). The code on master runs fine, so it must have been something in the 252 or 262 branches. I've done some initial work wrapping LOG.debug() calls in checks for LOG.isDebugEnabled(), but that didn't seem to help. All my tests were done with log4j.rootLogger=OFF, stdout I may have some more time to look at this tomorrow. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-222026212 Having thought about this, but not looked, my guess is that the arguments to logging functions are getting computed even if they're not printed, and that's costing lots of time. I'm not sure why that would affect multithreading though. @lewismc and @thammegowda, if you have any thoughts on any other changes you made that might have caused this, please let me know. (I think the lesson is that I should have let JOSHUA-252 get fully merged before pulling in the logging code. That would have let us separate these issues) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-222016587 Also, multithreading is broken. I randomly (not always) get the following error when decoding with more than 1 thread: ``` Exception in thread "Thread-4" java.lang.RuntimeException: Input 1: FATAL UNCAUGHT EXCEPTION: null at org.apache.joshua.decoder.Decoder$DecoderThreadRunner.run(Decoder.java:433) Caused by: java.util.ConcurrentModificationException at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:901) at java.util.ArrayList$Itr.next(ArrayList.java:851) at org.apache.joshua.decoder.chart_parser.DotChart.addDotItem(DotChart.java:362) at org.apache.joshua.decoder.chart_parser.DotChart.expandDotCell(DotChart.java:223) at org.apache.joshua.decoder.chart_parser.Chart.expand(Chart.java:585) at org.apache.joshua.decoder.DecoderThread.translate(DecoderThread.java:141) at org.apache.joshua.decoder.Decoder$DecoderThreadRunner.run(Decoder.java:424) ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-222016079 Okay, I have merged. The handful of test cases I checked passed, but there is bad news: the decoder is *significantly* slower, like by an order of magnitude. Tests that returned instantly before or took just a second or two now take 10â20. This is with log levels turned off. Those of you who have worked on this branch, do you have any guesses what the problem might be? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-222013844 Okay, cool. It doesn't seem to look for that file automatically, though; if I remove the -Dlog4j.configuration, it doesn't find the file you pointed to. So I am just having it hard-point to that path. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-222011444 Okay, the answer is that our slfj4 is using log4j12 as the backend. I had to add -Dlog4j.configuration=$JOSHUA/logging.properties to the invocation, and then put log4j.rootLogger=OFF (or info, etc). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-222008424 Can someone tell me how to turn off logging in slf4j? I have tried passing -Dorg.slf4j.Logger.defaultLogLevel=info but this does not seem to work. Extensive googling around and walking through the manual have not turned up anything. I've probably missed something. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-222003148 Yep, working through those. Making good progress on the build. Once I can verify that the test cases pass under the new build (so I can know I've done the merge correctly), I'll push it up. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-222000585 Nevermind, found Thamme's [old instructions](https://github.com/apache/incubator-joshua/pull/14#issue-155139645). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-221969513 Okay, cool, I got this merged and compiling. First, THANK YOU for all of this amazing work! This feels like a real milestone and it was fun to work through the merge. Before I push, I need to run the tests, though, and am not sure how to invoke the decoder. Can you provide some assistance here? I don't know maven very well, so if there are any maven commands to type first, that would also help to know. Basically, if you can get me to the point where I can type $ echo test | joshua and it returns 0 ||| test ||| tm_glue_0=1.000 ||| 0.000 then I can run my regression tests and push up. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request: JOSHUA-252 Make it possible to use ...
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/pull/12#issuecomment-221928486 There have been changes on master (JOSHUA-271 and JOSHUA-272), will try to get to this soon, probably not till tomorrow. Do you want me to be the one to merge master into JOSHUA-252? Then we can test and merge back into master. I think this might be our 6.1 release? For packages: - Is it possible to have Maven do git checkouts (on specific versions?) That's one route - Otherwise, we should just rely on the user to set them up. The only real problem is for BerkeleyLM and KenLM, since those are runtime. I don't have much experience in how to do this. But we could have a maven target that does a git checkout of a specific version, kind of like a submodule. - Thrax and GIZA (and fast_align) we can tell the user to set up. matt --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/19aadf0e240acb09c5b4336068e6368083f26bef#commitcomment-17620834 In src/joshua/decoder/Decoder.java: In src/joshua/decoder/Decoder.java on line 515: Yes, builder is a more appropriate design name, will rename. I also like the with*() naming. And yes, that's more logically sensible and contained. I'll give it a whirl and see if it passes the regression tests. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/19aadf0e240acb09c5b4336068e6368083f26bef#commitcomment-17620963 In src/joshua/decoder/Decoder.java: In src/joshua/decoder/Decoder.java on line 515: As for using decode() versus decodeAll(): you wouldn't get threading that way, since decode() blocks. Thought you probably have some way of doing this in mind. Do you have some idea of where the entry point is for the API? The way decodeAll works is, you pass it an object that can be iterated over, and as it receives translations, it outputs them. I'm not sure how to do multithreading with the API if the entry point is a blocked call to decode(), but probably there's a standard way of doing this that I don't know about? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/f2f82c38af9aebd28f9d27f685a2e99767a2e575#commitcomment-17620761 In src/joshua/decoder/hypergraph/KBestExtractor.java: In src/joshua/decoder/hypergraph/KBestExtractor.java on line 93: Uh, yeah, that's how I should have done it before. I even complained about the logic being in multiple places and had a bug. Will add that now. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/f2f82c38af9aebd28f9d27f685a2e99767a2e575#commitcomment-17620750 In src/joshua/decoder/Translations.java: In src/joshua/decoder/Translations.java on line 49: That's a very good thought. So really, we should throw away the hypergraph earlier on, even before things get queued up. I think that's the right way, but that means you have to push all your parameters down into the function, about what you're going to want on the output. Any idea how to do that cleanly? Just through a JoshuaConfiguration, or something better? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/f2f82c38af9aebd28f9d27f685a2e99767a2e575#commitcomment-17620554 In src/joshua/decoder/Decoder.java: In src/joshua/decoder/Decoder.java on line 39: No, not sure who added it --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/f2f82c38af9aebd28f9d27f685a2e99767a2e575#commitcomment-17620548 In src/joshua/decoder/Decoder.java: In src/joshua/decoder/Decoder.java on line 193: This is a bug, needs to be restored or further fixed --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/868b340949f324b12d810d03c09c25fd5877d3dc#commitcomment-17618958 Yes, I don't know how to run them. Is it an eclipse plugin or an ant target? If someone can tell me, I'll start running them. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/76eb9587a9c321d45a9560481429e2dffd918e4e#commitcomment-17618721 In src/joshua/decoder/ff/tm/format/HieroFormatReader.java: In src/joshua/decoder/ff/tm/format/HieroFormatReader.java on line 83: I started doing this, but it's touching too many files. I will make a note to do it after the 252 merge. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/f5adcdefd3e52c58b36713798c0830d1c42099e3#commitcomment-17618600 In src/joshua/tools/GrammarPacker.java: In src/joshua/tools/GrammarPacker.java on line 242: Okay. Currently it's doing it in the GrammarReaders, but it's a redundancy. I'll make a comment so it's clearer in the code. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-joshua pull request:
Github user mjpost commented on the pull request: https://github.com/apache/incubator-joshua/commit/f5adcdefd3e52c58b36713798c0830d1c42099e3#commitcomment-17618788 In src/joshua/tools/GrammarPacker.java: In src/joshua/tools/GrammarPacker.java on line 295: These are all fine ideas. I think they're the kind of thing that one of us should just pick off sometime we're in the code doing something else. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---