[
https://issues.apache.org/jira/browse/OPENNLP-1450?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17697434#comment-17697434
]
ASF GitHub Bot commented on OPENNLP-1450:
-----------------------------------------
mawiesne commented on code in PR #520:
URL: https://github.com/apache/opennlp/pull/520#discussion_r1127805957
##########
opennlp-tools/src/main/java/opennlp/tools/ml/maxent/quasinewton/QNMinimizer.java:
##########
@@ -226,8 +226,8 @@ public double[] minimize(Function function) {
if (logger.isDebugEnabled()) {
logger.debug("Solving convex optimization problem.");
logger.debug("Objective function has {} variable(s).", dimension);
- logger.debug("Performing " + iterations + " iterations with " +
- "L1Cost=" + l1Cost + " and L2Cost=" + l2Cost );
+ logger.debug("Performing {} iterations with L1Cost{}} and L2Cost={}",
Review Comment:
Instead of `L1Cost{}}` this should read `L1Cost={}`, so remove one `}` and
add `=`
##########
opennlp-tools/src/main/java/opennlp/tools/parser/treeinsert/Parser.java:
##########
@@ -305,9 +305,9 @@ else if (numNodes == 1) { //put sentence initial and final
punct in top node
buildModel.eval(buildContextGenerator.getContext(children,
advanceNodeIndex), bprobs);
double doneProb = bprobs[doneIndex];
if (logger.isDebugEnabled())
- logger.debug("adi=" + advanceNodeIndex + " " + advanceNode.getType() +
"."
- + advanceNode.getLabel() + " " + advanceNode + " choose build=" + (1
- doneProb)
- + " attach=" + doneProb);
+ logger.debug("adi={} {}.{} {} choose build={} attach={}",
+ advanceNode, advanceNode.getType(), advanceNode.getLabel(),
Review Comment:
Cave: first parameter should be `advanceNodeIndex` instead of `advanceNode`
(!)
##########
opennlp-tools/src/main/java/opennlp/tools/parser/treeinsert/Parser.java:
##########
@@ -465,21 +465,21 @@ else if (1 - cprobs[completeIndex] > probMass) { //just
incomplete advances
setIncomplete(updatedNode);
newParse2.addProb(StrictMath.log(1 - cprobs[completeIndex]));
if (logger.isDebugEnabled())
- logger.debug("Advancing both complete and incomplete
nodes; c="
- + cprobs[completeIndex]);
+ logger.debug("Advancing both complete and incomplete
nodes; c={}",
+ cprobs[completeIndex]);
}
}
} else {
if (logger.isDebugEnabled())
- logger.debug("Skipping " + fn.getType() + "." + fn.getLabel()
+ " "
- + fn + " daughter=" + (attachment ==
daughterAttachIndex)
- + " complete=" + isComplete(fn) + " prob=" + prob);
+ logger.debug("Skipping {}.{} {} daughter={} complete={}
prob={}",
+ fn.getType(), fn.getLabel(), fn, (attachment ==
daughterAttachIndex),
+ isComplete(fn), prob);
}
}
if (checkComplete && !isComplete(fn)) {
if (logger.isDebugEnabled())
- logger.debug("Stopping at incomplete node(" + fi + "): "
- + fn.getType() + "." + fn.getLabel() + " " + fn);
+ logger.debug("Stopping at incomplete node({}}): {} . {} {}",
Review Comment:
`node({}}):` should read `node({}):`
##########
opennlp-tools/src/main/java/opennlp/tools/ml/perceptron/SimplePerceptronSequenceTrainer.java:
##########
@@ -399,16 +398,16 @@ public void nextIteration(int iteration) throws
IOException {
predParams[oi] /= totIterations;
averageParams[pi].setParameter(oi, predParams[oi]);
if (logger.isTraceEnabled()) {
- logger.trace("updates[" + pi + "][" + oi + "]=(" +
updates[pi][oi][ITER] + ","
- + updates[pi][oi][EVENT] + "," + updates[pi][oi][VALUE] + ")
+ (" + iterations
- + "," + 0 + "," + params[pi].getParameters()[oi] + ") -> "
- + averageParams[pi].getParameters()[oi]);
+ logger.trace("updates[{}][{}]=({},{},{})({},{},{}= -> {}", pi,
oi, updates[pi][oi][ITER],
Review Comment:
`({},{},{})({},{},{}=` should read `({},{},{})({},{},{})`, note the `)` at
the end.
##########
opennlp-tools/src/main/java/opennlp/tools/ml/perceptron/SimplePerceptronSequenceTrainer.java:
##########
@@ -399,16 +398,16 @@ public void nextIteration(int iteration) throws
IOException {
predParams[oi] /= totIterations;
averageParams[pi].setParameter(oi, predParams[oi]);
if (logger.isTraceEnabled()) {
- logger.trace("updates[" + pi + "][" + oi + "]=(" +
updates[pi][oi][ITER] + ","
- + updates[pi][oi][EVENT] + "," + updates[pi][oi][VALUE] + ")
+ (" + iterations
- + "," + 0 + "," + params[pi].getParameters()[oi] + ") -> "
- + averageParams[pi].getParameters()[oi]);
+ logger.trace("updates[{}][{}]=({},{},{})({},{},{}= -> {}", pi,
oi, updates[pi][oi][ITER],
+ updates[pi][oi][EVENT], updates[pi][oi][VALUE], iterations,
0,
+ params[pi].getParameters()[oi],
averageParams[pi].getParameters()[oi]);
}
}
}
}
}
- logger.info("{}. (" + numCorrect + "/" + numEvents + ") " + ((double)
numCorrect / numEvents), iteration);
+ logger.info("{}. ({}/{}) {}", iteration, numCorrect,
+ numCorrect, ((double) numCorrect / numEvents));
Review Comment:
the 3rd parameter should not be `numCorrect` but `numEvents` (!)
> Revise log messages in OpenNLP
> ------------------------------
>
> Key: OPENNLP-1450
> URL: https://issues.apache.org/jira/browse/OPENNLP-1450
> Project: OpenNLP
> Issue Type: Sub-task
> Reporter: Richard Zowalla
> Assignee: Richard Zowalla
> Priority: Major
> Fix For: 2.1.1
>
>
> We should replace string concat with variable replacement provided by
> SLF4J-API and revise the log messages, if needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
