You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/04/27 11:19:46 UTC
[GitHub] [lucene] mocobeta opened a new pull request, #846: LUCENE-10493: move n-best logic to analysis-common
mocobeta opened a new pull request, #846:
URL: https://github.com/apache/lucene/pull/846
This is a follow-up of https://github.com/apache/lucene/pull/805 - NBest logic can separate from Japanese-specific stuff, and reside in analysis-common.
This doesn't reduce code duplication but tries to clarify the boundary between graph (or lattice) construction and language-specific tweaks. Also, this may allow KoreanTokenizer to support n-best as well as JapaneseTokenizer if the feature is worth adding to it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mocobeta merged pull request #846: LUCENE-10493: move n-best logic to analysis-common
Posted by GitBox <gi...@apache.org>.
mocobeta merged PR #846:
URL: https://github.com/apache/lucene/pull/846
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mocobeta commented on a diff in pull request #846: LUCENE-10493: move n-best logic to analysis-common
Posted by GitBox <gi...@apache.org>.
mocobeta commented on code in PR #846:
URL: https://github.com/apache/lucene/pull/846#discussion_r859684144
##########
lucene/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/ViterbiNBest.java:
##########
@@ -639,76 +622,35 @@ private void pruneAndRescore(int startPos, int endPos, int bestStartIDX) throws
posData,
pos,
toPos,
- posData.forwardID[forwardArcIDX],
+ posData.getForwardID(forwardArcIDX),
forwardType,
true);
}
}
- posData.forwardCount = 0;
+ posData.setForwardCount(0);
}
}
@Override
- protected void backtraceNBest(final Position endPosData, final boolean useEOS)
- throws IOException {
- if (lattice == null) {
- lattice = new Lattice();
- }
-
- final int endPos = endPosData.getPos();
- char[] fragment = buffer.get(lastBackTracePos, endPos - lastBackTracePos);
- lattice.setup(fragment, dictionaryMap, positions, lastBackTracePos, endPos, useEOS);
- lattice.markUnreachable();
- lattice.calcLeftCost(costs);
- lattice.calcRightCost(costs);
-
- int bestCost = lattice.bestCost();
- if (VERBOSE) {
- System.out.printf("DEBUG: 1-BEST COST: %d\n", bestCost);
- }
- for (int node : lattice.bestPathNodeList()) {
- registerNode(node, fragment);
- }
-
- for (int n = 2; ; ++n) {
- List<Integer> nbest = lattice.nBestNodeList(n);
- if (nbest.isEmpty()) {
- break;
- }
- int cost = lattice.cost(nbest.get(0));
- if (VERBOSE) {
- System.out.printf("DEBUG: %d-BEST COST: %d\n", n, cost);
- }
- if (bestCost + nBestCost < cost) {
- break;
- }
- for (int node : nbest) {
- registerNode(node, fragment);
- }
- }
- if (VERBOSE) {
- lattice.debugPrint();
- }
- }
-
- private void registerNode(int node, char[] fragment) {
- int left = lattice.nodeLeft[node];
- int right = lattice.nodeRight[node];
- TokenType type = lattice.nodeDicType[node];
+ protected void registerNode(int node, char[] fragment) {
Review Comment:
This has to remain in kuromoji for a similar reason as Viterbi's `backtrace()` has to remain in kuromoji and nori; in order to add a node to the graph, this relies on dictionary-specific features.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mocobeta commented on pull request #846: LUCENE-10493: move n-best logic to analysis-common
Posted by GitBox <gi...@apache.org>.
mocobeta commented on PR #846:
URL: https://github.com/apache/lucene/pull/846#issuecomment-1110967296
Passed all checks, and this is a safe refactoring that has no negative effect, I think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mocobeta commented on pull request #846: LUCENE-10493: move n-best logic to analysis-common
Posted by GitBox <gi...@apache.org>.
mocobeta commented on PR #846:
URL: https://github.com/apache/lucene/pull/846#issuecomment-1112127870
@mikemccand thank you for revewing!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mocobeta commented on a diff in pull request #846: LUCENE-10493: move n-best logic to analysis-common
Posted by GitBox <gi...@apache.org>.
mocobeta commented on code in PR #846:
URL: https://github.com/apache/lucene/pull/846#discussion_r859684144
##########
lucene/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/ViterbiNBest.java:
##########
@@ -639,76 +622,35 @@ private void pruneAndRescore(int startPos, int endPos, int bestStartIDX) throws
posData,
pos,
toPos,
- posData.forwardID[forwardArcIDX],
+ posData.getForwardID(forwardArcIDX),
forwardType,
true);
}
}
- posData.forwardCount = 0;
+ posData.setForwardCount(0);
}
}
@Override
- protected void backtraceNBest(final Position endPosData, final boolean useEOS)
- throws IOException {
- if (lattice == null) {
- lattice = new Lattice();
- }
-
- final int endPos = endPosData.getPos();
- char[] fragment = buffer.get(lastBackTracePos, endPos - lastBackTracePos);
- lattice.setup(fragment, dictionaryMap, positions, lastBackTracePos, endPos, useEOS);
- lattice.markUnreachable();
- lattice.calcLeftCost(costs);
- lattice.calcRightCost(costs);
-
- int bestCost = lattice.bestCost();
- if (VERBOSE) {
- System.out.printf("DEBUG: 1-BEST COST: %d\n", bestCost);
- }
- for (int node : lattice.bestPathNodeList()) {
- registerNode(node, fragment);
- }
-
- for (int n = 2; ; ++n) {
- List<Integer> nbest = lattice.nBestNodeList(n);
- if (nbest.isEmpty()) {
- break;
- }
- int cost = lattice.cost(nbest.get(0));
- if (VERBOSE) {
- System.out.printf("DEBUG: %d-BEST COST: %d\n", n, cost);
- }
- if (bestCost + nBestCost < cost) {
- break;
- }
- for (int node : nbest) {
- registerNode(node, fragment);
- }
- }
- if (VERBOSE) {
- lattice.debugPrint();
- }
- }
-
- private void registerNode(int node, char[] fragment) {
- int left = lattice.nodeLeft[node];
- int right = lattice.nodeRight[node];
- TokenType type = lattice.nodeDicType[node];
+ protected void registerNode(int node, char[] fragment) {
Review Comment:
This has to remain in kuromoji for a similar reason as Viterbi's `backtrace()` has to remain in kuromoji and nori; in order to add a node to the pending list, this relies on dictionary-specific features.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mocobeta commented on a diff in pull request #846: LUCENE-10493: move n-best logic to analysis-common
Posted by GitBox <gi...@apache.org>.
mocobeta commented on code in PR #846:
URL: https://github.com/apache/lucene/pull/846#discussion_r859684144
##########
lucene/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/ViterbiNBest.java:
##########
@@ -639,76 +622,35 @@ private void pruneAndRescore(int startPos, int endPos, int bestStartIDX) throws
posData,
pos,
toPos,
- posData.forwardID[forwardArcIDX],
+ posData.getForwardID(forwardArcIDX),
forwardType,
true);
}
}
- posData.forwardCount = 0;
+ posData.setForwardCount(0);
}
}
@Override
- protected void backtraceNBest(final Position endPosData, final boolean useEOS)
- throws IOException {
- if (lattice == null) {
- lattice = new Lattice();
- }
-
- final int endPos = endPosData.getPos();
- char[] fragment = buffer.get(lastBackTracePos, endPos - lastBackTracePos);
- lattice.setup(fragment, dictionaryMap, positions, lastBackTracePos, endPos, useEOS);
- lattice.markUnreachable();
- lattice.calcLeftCost(costs);
- lattice.calcRightCost(costs);
-
- int bestCost = lattice.bestCost();
- if (VERBOSE) {
- System.out.printf("DEBUG: 1-BEST COST: %d\n", bestCost);
- }
- for (int node : lattice.bestPathNodeList()) {
- registerNode(node, fragment);
- }
-
- for (int n = 2; ; ++n) {
- List<Integer> nbest = lattice.nBestNodeList(n);
- if (nbest.isEmpty()) {
- break;
- }
- int cost = lattice.cost(nbest.get(0));
- if (VERBOSE) {
- System.out.printf("DEBUG: %d-BEST COST: %d\n", n, cost);
- }
- if (bestCost + nBestCost < cost) {
- break;
- }
- for (int node : nbest) {
- registerNode(node, fragment);
- }
- }
- if (VERBOSE) {
- lattice.debugPrint();
- }
- }
-
- private void registerNode(int node, char[] fragment) {
- int left = lattice.nodeLeft[node];
- int right = lattice.nodeRight[node];
- TokenType type = lattice.nodeDicType[node];
+ protected void registerNode(int node, char[] fragment) {
Review Comment:
This has to remain in kuromoji for a similar reason as Viterbi's `backtrace()` has to remain in kuromoji and nori; in order to add a token to the pending list, this relies on dictionary-specific features.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org