You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@opennlp.apache.org by "NishantShri4 (via GitHub)" <gi...@apache.org> on 2025/03/24 07:01:38 UTC
[PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
NishantShri4 opened a new pull request, #758:
URL: https://github.com/apache/opennlp/pull/758
Thank you for contributing to Apache OpenNLP.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [x] Does your PR title start with OPENNLP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [x] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [x] Is your initial contribution a single, squashed commit?
### For code changes:
- [x] Have you ensured that the full suite of tests is executed via mvn clean install at the root opennlp folder?
- [ ] Have you written or updated unit tests to verify your changes?
- [] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file in opennlp folder?
- [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found in opennlp folder?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions for build issues and submit an update to your PR as soon as possible.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on PR #758:
URL: https://github.com/apache/opennlp/pull/758#issuecomment-2800174286
hi @rzo1, @mawiesne,
Hope you are well. I have tried to fix the review comments. Would you pls. be able to review once more and direct towards the intended solution? Many thanks in advance.
**Some queries and ToDos:**
1) The jira description gives following prototype of finishedTraining() method.
```
finishedTraining(int iterations, int numberCorrectEvents, int totalEvents, StopCriteria stopCriteria);
}
```
Please can you clarify, what is the use of numberCorrectEvents and totalEvents parameters? In my current implementation, I have not used them, instead I found stopCriteria is sufficient. Pls. take a look.
2) ToDo - The TPM & StopCriteria are currently only integrated with Perceptron and GIS Trainer. Integrate with other trainers also.
3) ToDo (if preferred) - The displayAndClear method could use a boolean parameter to control clearing of underlying data structure holding training progress.
4) ToDo : Update the documentation to include description of TPM and StopCriteria.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2043081606
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.function.Predicate;
+
+/**
+ * Stop-criteria for a {@link opennlp.tools.ml.model.AbstractModel} training .
+ */
+public interface StopCriteria extends Predicate<Double> {
Review Comment:
This can be made as `extends Predicate<Object>`;
So that this common API is flexible to receive any object as input to verify testing Criteria? views pls.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056096517
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/IterDeltaAccuracyUnderTolerance.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import opennlp.tools.ml.perceptron.PerceptronTrainer;
+import opennlp.tools.util.TrainingParameters;
+
+/**
+ * A {@link StopCriteria} implementation to identify whether the absolute
+ * difference between the training accuracy of current and previous iteration is under the defined tolerance.
+ */
+public class IterDeltaAccuracyUnderTolerance implements StopCriteria {
Review Comment:
Thanks. Done.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/DefaultTrainingProgressMonitor.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static opennlp.tools.monitoring.StopCriteria.FINISHED;
+
+/**
+ * An implementation of {@link TrainingProgressMonitor} which publishes model training progress to the chosen
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on PR #758:
URL: https://github.com/apache/opennlp/pull/758#issuecomment-2747126795
Hello @mawiesne / @kottmann . Good day!
Is this item waiting to be picked up? https://issues.apache.org/jira/browse/OPENNLP-124.
Here is my draft PR.
This PR is just to explain my current understanding of the Jira .
This needs further improvements (refactoring, integration with other model trainers, unit tests etc.).
I am happy to be part of the discussion and pick it up for implementation if the team approves.
Pls. let me know your views.
Attached the output of a couple of existing Tests (Perceptron Trainer) based on the integration with Console based TrainingProgressMonitor.
[PerceptroTrainerUnitTests-LogOutput.txt](https://github.com/user-attachments/files/19416144/PerceptroTrainerUnitTests-LogOutput.txt)
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2043081606
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.function.Predicate;
+
+/**
+ * Stop-criteria for a {@link opennlp.tools.ml.model.AbstractModel} training .
+ */
+public interface StopCriteria extends Predicate<Double> {
Review Comment:
This can be made as extends `Predicate<Object>`
So that this common API is flexible to receive any object as input to verify testing Criteria? views pls.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on PR #758:
URL: https://github.com/apache/opennlp/pull/758#issuecomment-2825295729
> Thx @NishantShri4 for providing this substantial contribution. I've left feedback by comments to further improve it. Once addressed, I'll re-check and potentially, @rzo1 can add his final thoughts/checks then.
Thanks very much @mawiesne for the detailed review earlier. Very useful. I have pushed changes earlier to answer/fix the review comments. I could see two approvals available now. Thanks to the approvers for their time.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2043680249
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
+ */
+
+public interface TrainingProgressMonitor {
+
+ /**
+ * Captures the Iteration progress.
+ *
+ * @param iteration The completed iteration number.
+ * @param numberCorrectEvents Number of correctly predicted events in this iteration.
+ * @param totalEvents Total count of events processed in this iteration.
+ * @param measure Measure used to quantify training success.
+ * @param measureValue measure value corresponding to the applicable {@link TrainingMeasure}.
+ */
+ void finishedIteration(int iteration, int numberCorrectEvents, int totalEvents,
+ TrainingMeasure measure, double measureValue);
+
+ /**
+ * Captures the Training completion progress.
+ *
+ * @param iterations Total number of iterations configured for training.
+ * @param stopCriteria Exit criteria for training.
+ */
+ void finishedTraining(int iterations, StopCriteria stopCriteria);
+
+ /**
+ * Checks whether the training has finished. *
+ * @return A boolean value to identify whether the training has finished.
+ */
+ boolean isTrainingFinished();
+
+ /**
+ * Display the Training progress and clear the underlying data structure
+ * used to store the training progress events. Callers of this method can invoke it periodically
+ * during training progress to avoid storing too much progress related data.
+ */
+ void displayAndClear();
Review Comment:
This can be made to accept a boolean input parameter which decides whether the underlying data structure is cleared or not?
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2041237556
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,10 @@
+package opennlp.tools.monitoring;
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056091241
##########
opennlp-tools/src/test/java/opennlp/tools/monitoring/LogLikelihoodThresholdBreachedTest.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.Map;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
+import opennlp.tools.util.TrainingParameters;
+
+import static opennlp.tools.ml.maxent.GISTrainer.LOG_LIKELIHOOD_THRESHOLD_PARAM;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+
+class LogLikelihoodThresholdBreachedTest {
+
+ private StopCriteria stopCriteria;
+
+ @BeforeEach
+ public void setup() {
+ stopCriteria = new LogLikelihoodThresholdBreached(
+ new TrainingParameters(Map.of(LOG_LIKELIHOOD_THRESHOLD_PARAM,5.)));
+ }
+
+ @ParameterizedTest()
Review Comment:
Thanks. Done.
##########
pom.xml:
##########
@@ -180,6 +180,7 @@
<!-- Dependency versions -->
<junit.version>5.12.1</junit.version>
<junit5-system-exit.version>2.0.2</junit5-system-exit.version>
+ <assertj-core.version>3.27.3</assertj-core.version>
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056111711
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.function.Predicate;
+
+/**
+ * Stop-criteria for a {@link opennlp.tools.ml.model.AbstractModel} training .
+ */
+public interface StopCriteria extends Predicate<Double> {
+
+ String FINISHED = "Training Finished after completing %s Iterations successfully.";
+
+ /**
+ * @return - A detailed message when the stop-criteria is satisfied during model training.
Review Comment:
Thanks. Done. Verified other javadocs too.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056110450
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
+ */
+
+public interface TrainingProgressMonitor {
+
+ /**
+ * Captures the Iteration progress.
+ *
+ * @param iteration The completed iteration number.
+ * @param numberCorrectEvents Number of correctly predicted events in this iteration.
+ * @param totalEvents Total count of events processed in this iteration.
+ * @param measure Measure used to quantify training success.
+ * @param measureValue measure value corresponding to the applicable {@link TrainingMeasure}.
+ */
+ void finishedIteration(int iteration, int numberCorrectEvents, int totalEvents,
+ TrainingMeasure measure, double measureValue);
+
+ /**
+ * Captures the Training completion progress.
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056098821
##########
opennlp-tools/src/test/java/opennlp/tools/ml/TrainerFactoryTest.java:
##########
@@ -78,4 +83,15 @@ void testIsSequenceTrainerFalse() {
Assertions.assertNotEquals(TrainerType.EVENT_MODEL_SEQUENCE_TRAINER, trainerType);
}
+ @Test
+ void testGetEventTrainerConfiguration() {
+ mlParams.put(TrainingParameters.ALGORITHM_PARAM, GISTrainer.MAXENT_VALUE);
+
+ TrainingConfiguration config = new TrainingConfiguration(new DefaultTrainingProgressMonitor(),
+ new LogLikelihoodThresholdBreached(mlParams));
+
+ AbstractTrainer trainer = (AbstractTrainer)TrainerFactory.getEventTrainer(mlParams, null, config);
+ assertTrue(trainer.getTrainingConfiguration().progMon() instanceof DefaultTrainingProgressMonitor);
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2009706730
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/ConsoleTrainingProgressMonitor.java:
##########
@@ -0,0 +1,51 @@
+package opennlp.tools.monitoring;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import static opennlp.tools.monitoring.StopCriteria.TRAINING_FINISHED_DEFAULT_MSG;
+
+/**
+ * Publishes ML model's Training progress to console.
+ */
+public class ConsoleTrainingProgressMonitor implements TrainingProgressMonitor {
+
+ private static final Logger logger = LoggerFactory.getLogger(ConsoleTrainingProgressMonitor.class);
+
+ /**
+ * Keeps a track whether training was already finished because StopCriteria was met.
+ */
+ volatile boolean isTrainingFinished;
Review Comment:
Can this be `private`?
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/PrevNIterationAccuracyLessThanTolerance.java:
##########
@@ -0,0 +1,30 @@
+package opennlp.tools.monitoring;
Review Comment:
License header
##########
opennlp-tools/src/main/java/opennlp/tools/ml/perceptron/PerceptronTrainer.java:
##########
@@ -257,15 +260,17 @@ public AbstractModel trainModel(int iterations, DataIndexer di, int cutoff, bool
logger.info("Computing model parameters...");
- MutableContext[] finalParameters = findParameters(iterations, useAverage);
+
+ //@TODO : Ideally an instance of TrainingProgressMonitor will be introduced via Trainer Factory.
+ MutableContext[] finalParameters = findParameters(iterations, useAverage, new ConsoleTrainingProgressMonitor());
Review Comment:
I agree, that a TPM should be created via the factory. Could also be a part of the training configuration (which TPM impl to use and which stop criteria to apply)
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,10 @@
+package opennlp.tools.monitoring;
+
+/** Used to identify the stop criteria for {@link opennlp.tools.ml.model.AbstractModel} training.*/
+public interface StopCriteria {
+
+ String TRAINING_FINISHED_DEFAULT_MSG = "Training Finished after completing %s Iterations successfully.";
Review Comment:
`finished`
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/ConsoleTrainingProgressMonitor.java:
##########
@@ -0,0 +1,51 @@
+package opennlp.tools.monitoring;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import static opennlp.tools.monitoring.StopCriteria.TRAINING_FINISHED_DEFAULT_MSG;
+
+/**
+ * Publishes ML model's Training progress to console.
+ */
+public class ConsoleTrainingProgressMonitor implements TrainingProgressMonitor {
+
+ private static final Logger logger = LoggerFactory.getLogger(ConsoleTrainingProgressMonitor.class);
+
+ /**
+ * Keeps a track whether training was already finished because StopCriteria was met.
+ */
+ volatile boolean isTrainingFinished;
+
+ private final List<String> progress = new LinkedList<>();
Review Comment:
We might want to add an API possibility to clear this list? Otherwise, it might grow and grow really large?
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/ConsoleTrainingProgressMonitor.java:
##########
@@ -0,0 +1,51 @@
+package opennlp.tools.monitoring;
Review Comment:
Missing license header.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/PrevNIterationAccuracyLessThanTolerance.java:
##########
@@ -0,0 +1,30 @@
+package opennlp.tools.monitoring;
+
+/** Identifies whether the difference between the training accuracy for current iteration
+ * and the training accuracy of previous
+ * n-1, n-2.. n-ith iterations is less than the defined Tolerance.
+ *
+ */
+public class PrevNIterationAccuracyLessThanTolerance implements StopCriteria {
+
+ public static String ACCURACY_DIFF_UNDER_TOLERANCE = "Stopping: change in training set accuracy less than {%s}";
+
+ private final double tolerance;
+
+ /** TODO: 24-03-2025 : i) Should a generic test method be exposed at interface level?
+ * ii) iterationDeltaAccuracy could be a list?*/
+ public boolean test (double ... iterationDeltaAccuracy) {
Review Comment:
This would also need some more javadoc to clarify intend and possible arguments.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,14 @@
+package opennlp.tools.monitoring;
Review Comment:
License header
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,10 @@
+package opennlp.tools.monitoring;
Review Comment:
License header
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/ConsoleTrainingProgressMonitor.java:
##########
@@ -0,0 +1,51 @@
+package opennlp.tools.monitoring;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import static opennlp.tools.monitoring.StopCriteria.TRAINING_FINISHED_DEFAULT_MSG;
+
+/**
+ * Publishes ML model's Training progress to console.
+ */
+public class ConsoleTrainingProgressMonitor implements TrainingProgressMonitor {
Review Comment:
It depends on the logger configuration, so we might need to come up with a better name for it.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/PrevNIterationAccuracyLessThanTolerance.java:
##########
@@ -0,0 +1,30 @@
+package opennlp.tools.monitoring;
+
+/** Identifies whether the difference between the training accuracy for current iteration
+ * and the training accuracy of previous
+ * n-1, n-2.. n-ith iterations is less than the defined Tolerance.
+ *
+ */
+public class PrevNIterationAccuracyLessThanTolerance implements StopCriteria {
+
+ public static String ACCURACY_DIFF_UNDER_TOLERANCE = "Stopping: change in training set accuracy less than {%s}";
+
+ private final double tolerance;
+
+ /** TODO: 24-03-2025 : i) Should a generic test method be exposed at interface level?
Review Comment:
I think, that we should have a generic method at interface level and impl. can override.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2041232771
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/ConsoleTrainingProgressMonitor.java:
##########
@@ -0,0 +1,51 @@
+package opennlp.tools.monitoring;
Review Comment:
Thanks. This file is renamed to DefaultTrainingProgressMonitor and License header is added.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/ConsoleTrainingProgressMonitor.java:
##########
@@ -0,0 +1,51 @@
+package opennlp.tools.monitoring;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import static opennlp.tools.monitoring.StopCriteria.TRAINING_FINISHED_DEFAULT_MSG;
+
+/**
+ * Publishes ML model's Training progress to console.
+ */
+public class ConsoleTrainingProgressMonitor implements TrainingProgressMonitor {
Review Comment:
Thanks. This file is renamed to DefaultTrainingProgressMonitor and License header is added.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2041238177
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,10 @@
+package opennlp.tools.monitoring;
+
+/** Used to identify the stop criteria for {@link opennlp.tools.ml.model.AbstractModel} training.*/
+public interface StopCriteria {
+
+ String TRAINING_FINISHED_DEFAULT_MSG = "Training Finished after completing %s Iterations successfully.";
Review Comment:
Thanks. Done (Variable name is updated).
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056079066
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
+ */
+
+public interface TrainingProgressMonitor {
+
+ /**
+ * Captures the Iteration progress.
+ *
+ * @param iteration The completed iteration number.
+ * @param numberCorrectEvents Number of correctly predicted events in this iteration.
+ * @param totalEvents Total count of events processed in this iteration.
+ * @param measure Measure used to quantify training success.
+ * @param measureValue measure value corresponding to the applicable {@link TrainingMeasure}.
+ */
+ void finishedIteration(int iteration, int numberCorrectEvents, int totalEvents,
+ TrainingMeasure measure, double measureValue);
+
+ /**
+ * Captures the Training completion progress.
+ *
+ * @param iterations Total number of iterations configured for training.
+ * @param stopCriteria Exit criteria for training.
+ */
+ void finishedTraining(int iterations, StopCriteria stopCriteria);
+
+ /**
+ * Checks whether the training has finished. *
+ * @return A boolean value to identify whether the training has finished.
+ */
+ boolean isTrainingFinished();
+
+ /**
+ * Display the Training progress and clear the underlying data structure
+ * used to store the training progress events. Callers of this method can invoke it periodically
+ * during training progress to avoid storing too much progress related data.
+ */
+ void displayAndClear();
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056729011
##########
opennlp-tools/src/main/java/opennlp/tools/commons/Trainer.java:
##########
@@ -35,4 +36,14 @@ public interface Trainer {
*/
void init(TrainingParameters trainParams, Map<String, String> reportMap);
+ /**
+ * Conducts the initialization of a {@link Trainer} via
+ * {@link TrainingParameters}, {@link Map report map} and {@link TrainingConfiguration}
+ *
+ * @param trainParams The {@link TrainingParameters} to use.
+ * @param reportMap The {@link Map} instance used as report map.
+ * @param config The {@link TrainingConfiguration} to use.
Review Comment:
Hello. Training configuration is allowed to be null. This allows handling following scenarios.
ii) Training models via command line. In this case there isn't any offering currently to provide the TrainingConfiguration via command line.
ii) Trainers are instantiated without using TrainerFactory. This happens in some tests e.g. PerceptronPrepAttachTest.testPerceptronOnPrepAttachData(...).
In both of the above scenarios, if TrainingConfiguration is not provided, the implementation provides default values for it (i.e. by constituting it using DefaultTrainingProgressMonitor and the default stop criteria (identified based on the trainer)).
I have updated the javadoc like below to reflect this.
` @param config The {@link TrainingConfiguration} to use. If null, suitable defaults will be used.`
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056095512
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/DefaultTrainingProgressMonitor.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static opennlp.tools.monitoring.StopCriteria.FINISHED;
+
+/**
+ * An implementation of {@link TrainingProgressMonitor} which publishes model training progress to the chosen
+ * logging destination.
+ */
+public class DefaultTrainingProgressMonitor implements TrainingProgressMonitor {
+
+ private static final Logger logger = LoggerFactory.getLogger(DefaultTrainingProgressMonitor.class);
+
+ /**
+ * Keeps a track whether training was already finished because StopCriteria was met.
Review Comment:
Thanks. Done. Comment is revised now.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2012703736
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/ConsoleTrainingProgressMonitor.java:
##########
@@ -0,0 +1,51 @@
+package opennlp.tools.monitoring;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import static opennlp.tools.monitoring.StopCriteria.TRAINING_FINISHED_DEFAULT_MSG;
+
+/**
+ * Publishes ML model's Training progress to console.
+ */
+public class ConsoleTrainingProgressMonitor implements TrainingProgressMonitor {
Review Comment:
Thanks @rzo1.
How about - DefaultTrainingProgressMonitor? This will route the messages to whatever logging INFO appender is configured. Or pls. feel free to suggest another suitable name.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2041237348
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/PrevNIterationAccuracyLessThanTolerance.java:
##########
@@ -0,0 +1,30 @@
+package opennlp.tools.monitoring;
Review Comment:
Thanks. This file is removed. Instead IterDeltaAccuracyUnderTolerance is added with Javadocs to express its intent.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056085027
##########
opennlp-tools/src/main/java/opennlp/tools/ml/AbstractTrainer.java:
##########
@@ -55,6 +57,20 @@ public void init(TrainingParameters trainParams, Map<String,String> reportMap) {
this.reportMap = reportMap;
}
+ /**
+ * Initializes a {@link AbstractTrainer} using following parameters.
+ *
+ * @param trainParams The {@link TrainingParameters} to use.
+ * @param reportMap The {@link Map} instance used as report map.
+ * @param config The {@link TrainingConfiguration} to use.
Review Comment:
Thanks. Done (Made a change to inherit javadocs from parent method.)
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056107671
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.function.Predicate;
+
+/**
+ * Stop-criteria for a {@link opennlp.tools.ml.model.AbstractModel} training .
Review Comment:
Thanks. Done.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on PR #758:
URL: https://github.com/apache/opennlp/pull/758#issuecomment-2830169450
Thanks again (and of course, we truly appreciate your contribution)! We’ll go ahead and merge this PR once we get confirmation from the secretary.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on PR #758:
URL: https://github.com/apache/opennlp/pull/758#issuecomment-2810259379
Hi Reviewers - All checks are green now. This is available for review. If possible, pls. take a look.
One query - This PR has 10 commits. Before presenting it for review, Is it needed to club them into a single commit?
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on PR #758:
URL: https://github.com/apache/opennlp/pull/758#issuecomment-2810401172
> Hi Reviewers - All checks are green now. This is available for review. If possible, pls. take a look.
>
> One query - This PR has 10 commits. Before presenting it for review, Is it needed to club them into a single commit?
Thx @NishantShri4 for moving this topic forward! Could you squash those commits and force push the resulting single commit? Once available, we'll have a detailed look and provide feedback.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2052101545
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
+ */
+
+public interface TrainingProgressMonitor {
+
+ /**
+ * Captures the Iteration progress.
+ *
+ * @param iteration The completed iteration number.
+ * @param numberCorrectEvents Number of correctly predicted events in this iteration.
+ * @param totalEvents Total count of events processed in this iteration.
+ * @param measure Measure used to quantify training success.
+ * @param measureValue measure value corresponding to the applicable {@link TrainingMeasure}.
+ */
+ void finishedIteration(int iteration, int numberCorrectEvents, int totalEvents,
+ TrainingMeasure measure, double measureValue);
+
+ /**
+ * Captures the Training completion progress.
+ *
+ * @param iterations Total number of iterations configured for training.
+ * @param stopCriteria Exit criteria for training.
+ */
+ void finishedTraining(int iterations, StopCriteria stopCriteria);
+
+ /**
+ * Checks whether the training has finished. *
+ * @return A boolean value to identify whether the training has finished.
+ */
+ boolean isTrainingFinished();
+
+ /**
+ * Display the Training progress and clear the underlying data structure
+ * used to store the training progress events. Callers of this method can invoke it periodically
+ * during training progress to avoid storing too much progress related data.
+ */
+ void displayAndClear();
Review Comment:
If parameter `clear` is introduced, method should be renamed to `void display(boolean clear)`
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056014469
##########
opennlp-tools/src/main/java/opennlp/tools/ml/maxent/GISTrainer.java:
##########
@@ -709,4 +727,25 @@ synchronized double getLoglikelihood() {
return loglikelihood;
}
}
+
+ /**
+ * Get the {@link StopCriteria} associated with this Trainer.
+ * @param trainingConfig - If {@link TrainingConfiguration} is null or
+ * {@link TrainingConfiguration#stopCriteria()} is null then return a default {@link StopCriteria}.
+ */
+ private StopCriteria getStopCriteria(TrainingConfiguration trainingConfig) {
+ return trainingConfig != null && trainingConfig.stopCriteria() != null
+ ? trainingConfig.stopCriteria() : new LogLikelihoodThresholdBreached(trainingParameters);
+ }
+
+ /**
+ * Get the {@link TrainingProgressMonitor} associated with this Trainer.
+ * @param trainingConfig If {@link TrainingConfiguration} is null or
+ * {@link TrainingConfiguration#progMon()} is null then return a default {@link TrainingProgressMonitor}.
+ */
Review Comment:
Thanks Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056077189
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.function.Predicate;
+
+/**
+ * Stop-criteria for a {@link opennlp.tools.ml.model.AbstractModel} training .
+ */
+public interface StopCriteria extends Predicate<Double> {
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056089215
##########
opennlp-tools/src/main/java/opennlp/tools/ml/TrainerFactory.java:
##########
@@ -180,6 +182,22 @@ public static <T> EventModelSequenceTrainer<T> getEventModelSequenceTrainer(
}
}
+ /**
+ * Works like {@link TrainerFactory#getEventTrainer(TrainingParameters, Map, TrainingConfiguration)}
+ * except that the {@link TrainingConfiguration} is initialized with {@link DefaultTrainingProgressMonitor}
+ * and a null {@link opennlp.tools.monitoring.StopCriteria}.
+ * If not provided, the actual {@link opennlp.tools.monitoring.StopCriteria}
+ * will be decided by the {@link EventTrainer} implementation.
+ *
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on PR #758:
URL: https://github.com/apache/opennlp/pull/758#issuecomment-2747145993
FYI: @jzonthemtn + @rzo1
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2043081606
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.function.Predicate;
+
+/**
+ * Stop-criteria for a {@link opennlp.tools.ml.model.AbstractModel} training .
+ */
+public interface StopCriteria extends Predicate<Double> {
Review Comment:
This can be made as extends Predicate<Object>
So that this common API is flexible to receive any object as input to verify testing Criteria? views pls.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2041237463
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/PrevNIterationAccuracyLessThanTolerance.java:
##########
@@ -0,0 +1,30 @@
+package opennlp.tools.monitoring;
+
+/** Identifies whether the difference between the training accuracy for current iteration
+ * and the training accuracy of previous
+ * n-1, n-2.. n-ith iterations is less than the defined Tolerance.
+ *
+ */
+public class PrevNIterationAccuracyLessThanTolerance implements StopCriteria {
+
+ public static String ACCURACY_DIFF_UNDER_TOLERANCE = "Stopping: change in training set accuracy less than {%s}";
+
+ private final double tolerance;
+
+ /** TODO: 24-03-2025 : i) Should a generic test method be exposed at interface level?
Review Comment:
Thanks. Done. Pls. refer to StopCriteria Interface.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/PrevNIterationAccuracyLessThanTolerance.java:
##########
@@ -0,0 +1,30 @@
+package opennlp.tools.monitoring;
+
+/** Identifies whether the difference between the training accuracy for current iteration
+ * and the training accuracy of previous
+ * n-1, n-2.. n-ith iterations is less than the defined Tolerance.
+ *
+ */
+public class PrevNIterationAccuracyLessThanTolerance implements StopCriteria {
+
+ public static String ACCURACY_DIFF_UNDER_TOLERANCE = "Stopping: change in training set accuracy less than {%s}";
+
+ private final double tolerance;
+
+ /** TODO: 24-03-2025 : i) Should a generic test method be exposed at interface level?
+ * ii) iterationDeltaAccuracy could be a list?*/
+ public boolean test (double ... iterationDeltaAccuracy) {
Review Comment:
Thanks. This file is removed. Instead Added IterDeltaAccuracyUnderTolerance class with its intent in the JavaDocs.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2041237249
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/ConsoleTrainingProgressMonitor.java:
##########
@@ -0,0 +1,51 @@
+package opennlp.tools.monitoring;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import static opennlp.tools.monitoring.StopCriteria.TRAINING_FINISHED_DEFAULT_MSG;
+
+/**
+ * Publishes ML model's Training progress to console.
+ */
+public class ConsoleTrainingProgressMonitor implements TrainingProgressMonitor {
+
+ private static final Logger logger = LoggerFactory.getLogger(ConsoleTrainingProgressMonitor.class);
+
+ /**
+ * Keeps a track whether training was already finished because StopCriteria was met.
+ */
+ volatile boolean isTrainingFinished;
+
+ private final List<String> progress = new LinkedList<>();
Review Comment:
Thanks. Added a displayAndClear() method, which takes care of emptying the underlying data structure.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2052099150
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.function.Predicate;
+
+/**
+ * Stop-criteria for a {@link opennlp.tools.ml.model.AbstractModel} training .
+ */
+public interface StopCriteria extends Predicate<Double> {
Review Comment:
@NishantShri4
Or even nicer
`public interface StopCriteria<T extends Number> extends Predicate<T> {`
This way, several numerical stop criteria can be defined, either as integer/long or float/double precision.
Don't think that we need sth different than `Number` as base type for `T`, do we?
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056095512
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/DefaultTrainingProgressMonitor.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static opennlp.tools.monitoring.StopCriteria.FINISHED;
+
+/**
+ * An implementation of {@link TrainingProgressMonitor} which publishes model training progress to the chosen
+ * logging destination.
+ */
+public class DefaultTrainingProgressMonitor implements TrainingProgressMonitor {
+
+ private static final Logger logger = LoggerFactory.getLogger(DefaultTrainingProgressMonitor.class);
+
+ /**
+ * Keeps a track whether training was already finished because StopCriteria was met.
Review Comment:
Thanks. Done. Javadoc is revised now.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056099275
##########
opennlp-tools/src/test/java/opennlp/tools/monitoring/DefaultTrainingProgressMonitorTest.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.slf4j.LoggerFactory;
+
+import opennlp.tools.util.TrainingParameters;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class DefaultTrainingProgressMonitorTest {
+
+ private static final String LOGGER_NAME = "opennlp";
+ private TrainingProgressMonitor progressMonitor;
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056108842
##########
opennlp-tools/src/main/java/opennlp/tools/util/TrainingConfiguration.java:
##########
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.util;
+
+import opennlp.tools.monitoring.StopCriteria;
+import opennlp.tools.monitoring.TrainingProgressMonitor;
+
+/**
+ * Training Configuration used in {@link opennlp.tools.ml.model.AbstractModel} training.
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056090013
##########
opennlp-tools/src/test/java/opennlp/tools/monitoring/IterDeltaAccuracyUnderToleranceTest.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.Map;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
+import opennlp.tools.util.TrainingParameters;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class IterDeltaAccuracyUnderToleranceTest {
+
+ private StopCriteria stopCriteria;
+
+ @BeforeEach
+ public void setup() {
+ stopCriteria = new IterDeltaAccuracyUnderTolerance(new TrainingParameters(Map.of("Tolerance",
+ .00002)));
+ }
+
+ @ParameterizedTest()
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056093623
##########
opennlp-tools/src/main/java/opennlp/tools/ml/TrainerFactory.java:
##########
@@ -189,11 +207,14 @@ public static <T> EventModelSequenceTrainer<T> getEventModelSequenceTrainer(
* {@link GISTrainer#MAXENT_VALUE} will be used.
* @param reportMap A {@link Map} that shall be used during initialization of
* the {@link EventTrainer}.
+ * @param config The {@link TrainingConfiguration} to be used. This determines the type of
+ * {@link opennlp.tools.monitoring.TrainingProgressMonitor}
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2058602487
##########
opennlp-tools/src/main/java/opennlp/tools/commons/Trainer.java:
##########
@@ -35,4 +36,14 @@ public interface Trainer {
*/
void init(TrainingParameters trainParams, Map<String, String> reportMap);
+ /**
+ * Conducts the initialization of a {@link Trainer} via
+ * {@link TrainingParameters}, {@link Map report map} and {@link TrainingConfiguration}
+ *
+ * @param trainParams The {@link TrainingParameters} to use.
+ * @param reportMap The {@link Map} instance used as report map.
+ * @param config The {@link TrainingConfiguration} to use.
Review Comment:
Thx for reporting back on this - I now better see the case for `null` as trainer configuration. The JavaDoc is for that seems helpful and clarifies that.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2041232930
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/ConsoleTrainingProgressMonitor.java:
##########
@@ -0,0 +1,51 @@
+package opennlp.tools.monitoring;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import static opennlp.tools.monitoring.StopCriteria.TRAINING_FINISHED_DEFAULT_MSG;
+
+/**
+ * Publishes ML model's Training progress to console.
+ */
+public class ConsoleTrainingProgressMonitor implements TrainingProgressMonitor {
+
+ private static final Logger logger = LoggerFactory.getLogger(ConsoleTrainingProgressMonitor.class);
+
+ /**
+ * Keeps a track whether training was already finished because StopCriteria was met.
+ */
+ volatile boolean isTrainingFinished;
Review Comment:
Thanks. Done in the new class i.e. DefaultTrainingProgressMonitor.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056106437
##########
opennlp-tools/src/main/java/opennlp/tools/ml/maxent/GISTrainer.java:
##########
@@ -709,4 +727,25 @@ synchronized double getLoglikelihood() {
return loglikelihood;
}
}
+
+ /**
+ * Get the {@link StopCriteria} associated with this Trainer.
+ * @param trainingConfig - If {@link TrainingConfiguration} is null or
+ * {@link TrainingConfiguration#stopCriteria()} is null then return a default {@link StopCriteria}.
+ */
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056108261
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
+ */
+
+public interface TrainingProgressMonitor {
+
+ /**
+ * Captures the Iteration progress.
+ *
+ * @param iteration The completed iteration number.
+ * @param numberCorrectEvents Number of correctly predicted events in this iteration.
+ * @param totalEvents Total count of events processed in this iteration.
+ * @param measure Measure used to quantify training success.
+ * @param measureValue measure value corresponding to the applicable {@link TrainingMeasure}.
+ */
+ void finishedIteration(int iteration, int numberCorrectEvents, int totalEvents,
+ TrainingMeasure measure, double measureValue);
+
+ /**
+ * Captures the Training completion progress.
+ *
+ * @param iterations Total number of iterations configured for training.
+ * @param stopCriteria Exit criteria for training.
+ */
+ void finishedTraining(int iterations, StopCriteria stopCriteria);
+
+ /**
+ * Checks whether the training has finished. *
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056107171
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/LogLikelihoodThresholdBreached.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import opennlp.tools.util.TrainingParameters;
+
+import static opennlp.tools.ml.maxent.GISTrainer.LOG_LIKELIHOOD_THRESHOLD_DEFAULT;
+import static opennlp.tools.ml.maxent.GISTrainer.LOG_LIKELIHOOD_THRESHOLD_PARAM;
+
+/**
+ * A {@link StopCriteria} implementation to identify whether the
+ * difference between the log likelihood of current and previous iteration is under the defined threshold.
+ */
+public class LogLikelihoodThresholdBreached implements StopCriteria {
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2052106714
##########
opennlp-tools/src/main/java/opennlp/tools/ml/AbstractTrainer.java:
##########
@@ -55,6 +57,20 @@ public void init(TrainingParameters trainParams, Map<String,String> reportMap) {
this.reportMap = reportMap;
}
+ /**
+ * Initializes a {@link AbstractTrainer} using following parameters.
+ *
+ * @param trainParams The {@link TrainingParameters} to use.
+ * @param reportMap The {@link Map} instance used as report map.
+ * @param config The {@link TrainingConfiguration} to use.
Review Comment:
See comment for `Trainer` interface.
##########
opennlp-tools/src/main/java/opennlp/tools/commons/Trainer.java:
##########
@@ -35,4 +36,14 @@ public interface Trainer {
*/
void init(TrainingParameters trainParams, Map<String, String> reportMap);
+ /**
+ * Conducts the initialization of a {@link Trainer} via
+ * {@link TrainingParameters}, {@link Map report map} and {@link TrainingConfiguration}
+ *
+ * @param trainParams The {@link TrainingParameters} to use.
+ * @param reportMap The {@link Map} instance used as report map.
+ * @param config The {@link TrainingConfiguration} to use.
Review Comment:
The training configuration should not be null at runtime, I guess?
Pls therefore add:
"Must not be {@code null}." to the parameter JavaDoc.
Same applies for different scenarios, pls re-iterate JavaDoc with that idea in mind.
##########
opennlp-tools/src/main/java/opennlp/tools/ml/perceptron/PerceptronTrainer.java:
##########
@@ -432,4 +450,25 @@ private static boolean isPerfectSquare(int n) {
return root * root == n;
}
+ /**
+ * Get the {@link StopCriteria} associated with this Trainer.
+ * @param trainingConfig - If {@link TrainingConfiguration} is null or
+ * {@link TrainingConfiguration#stopCriteria()} is null then return a default {@link StopCriteria}.
+ */
+ private StopCriteria getStopCriteria(TrainingConfiguration trainingConfig) {
+ return trainingConfig != null && trainingConfig.stopCriteria() != null
+ ? trainingConfig.stopCriteria() : new IterDeltaAccuracyUnderTolerance(trainingParameters);
+ }
+
+ /**
+ * Get the {@link TrainingProgressMonitor} associated with this Trainer.
+ * @param trainingConfig - If {@link TrainingConfiguration} is null or
+ * {@link TrainingConfiguration#progMon()}is null then
+ * return the default {@link TrainingProgressMonitor}.
+ */
Review Comment:
Pls link with `Trainer` interface via `{@link ..}`
Pls add specific return value JavaDoc.
Pls format `null` as `{@code null}`, twice here.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/DefaultTrainingProgressMonitor.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static opennlp.tools.monitoring.StopCriteria.FINISHED;
+
+/**
+ * An implementation of {@link TrainingProgressMonitor} which publishes model training progress to the chosen
+ * logging destination.
+ */
+public class DefaultTrainingProgressMonitor implements TrainingProgressMonitor {
+
+ private static final Logger logger = LoggerFactory.getLogger(DefaultTrainingProgressMonitor.class);
+
+ /**
+ * Keeps a track whether training was already finished because StopCriteria was met.
+ */
+ private volatile boolean isTrainingFinished;
+
+ /**
+ * An underlying list to capture training progress events.
+ */
+ private final List<String> progress;
+
+ public DefaultTrainingProgressMonitor() {
+ this.progress = new LinkedList<>();
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public synchronized void finishedIteration(int iteration, int numberCorrectEvents, int totalEvents,
+ TrainingMeasure measure, double measureValue) {
+ progress.add(String.format("%s: (%s/%s) %s : %s", iteration, numberCorrectEvents, totalEvents,
+ measure.getMeasureName(), measureValue));
+ }
+
+ @Override
+ public synchronized void finishedTraining(int iterations, StopCriteria stopCriteria) {
Review Comment:
Pls add, `{@inheritDoc}` as above, consistently.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
+ */
+
+public interface TrainingProgressMonitor {
+
+ /**
+ * Captures the Iteration progress.
+ *
+ * @param iteration The completed iteration number.
+ * @param numberCorrectEvents Number of correctly predicted events in this iteration.
+ * @param totalEvents Total count of events processed in this iteration.
+ * @param measure Measure used to quantify training success.
Review Comment:
Pls {@link ...} to TrainingMeasure here.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
+ */
+
+public interface TrainingProgressMonitor {
+
+ /**
+ * Captures the Iteration progress.
+ *
+ * @param iteration The completed iteration number.
+ * @param numberCorrectEvents Number of correctly predicted events in this iteration.
+ * @param totalEvents Total count of events processed in this iteration.
+ * @param measure Measure used to quantify training success.
+ * @param measureValue measure value corresponding to the applicable {@link TrainingMeasure}.
+ */
+ void finishedIteration(int iteration, int numberCorrectEvents, int totalEvents,
+ TrainingMeasure measure, double measureValue);
+
+ /**
+ * Captures the Training completion progress.
+ *
+ * @param iterations Total number of iterations configured for training.
+ * @param stopCriteria Exit criteria for training.
+ */
+ void finishedTraining(int iterations, StopCriteria stopCriteria);
+
+ /**
+ * Checks whether the training has finished. *
+ * @return A boolean value to identify whether the training has finished.
Review Comment:
Is boolean, obviously.
Better:
`{@code true} if the training has finished, {@code false} if the training is not yet completed.`
##########
opennlp-tools/src/test/java/opennlp/tools/monitoring/DefaultTrainingProgressMonitorTest.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
+import org.assertj.core.api.Assertions;
Review Comment:
Try to eliminate dependance on `org.assertj.core...` here and elsewhere.
##########
opennlp-tools/src/test/java/opennlp/tools/ml/TrainerFactoryTest.java:
##########
@@ -78,4 +83,15 @@ void testIsSequenceTrainerFalse() {
Assertions.assertNotEquals(TrainerType.EVENT_MODEL_SEQUENCE_TRAINER, trainerType);
}
+ @Test
+ void testGetEventTrainerConfiguration() {
+ mlParams.put(TrainingParameters.ALGORITHM_PARAM, GISTrainer.MAXENT_VALUE);
+
+ TrainingConfiguration config = new TrainingConfiguration(new DefaultTrainingProgressMonitor(),
+ new LogLikelihoodThresholdBreached(mlParams));
+
+ AbstractTrainer trainer = (AbstractTrainer)TrainerFactory.getEventTrainer(mlParams, null, config);
+ assertTrue(trainer.getTrainingConfiguration().progMon() instanceof DefaultTrainingProgressMonitor);
Review Comment:
Pls remove extra whitespace here and in line below after `instanceof` keyword.
##########
opennlp-tools/src/main/java/opennlp/tools/ml/TrainerFactory.java:
##########
@@ -189,11 +207,14 @@ public static <T> EventModelSequenceTrainer<T> getEventModelSequenceTrainer(
* {@link GISTrainer#MAXENT_VALUE} will be used.
* @param reportMap A {@link Map} that shall be used during initialization of
* the {@link EventTrainer}.
+ * @param config The {@link TrainingConfiguration} to be used. This determines the type of
+ * {@link opennlp.tools.monitoring.TrainingProgressMonitor}
Review Comment:
Please add an import for TrainingProgressMonitor and StopCriteria in TrainerFactory.
This way, we keep JavaDoc more compact here, avoiding fully qualified package names.
##########
opennlp-tools/src/main/java/opennlp/tools/ml/maxent/GISTrainer.java:
##########
@@ -709,4 +727,25 @@ synchronized double getLoglikelihood() {
return loglikelihood;
}
}
+
+ /**
+ * Get the {@link StopCriteria} associated with this Trainer.
+ * @param trainingConfig - If {@link TrainingConfiguration} is null or
+ * {@link TrainingConfiguration#stopCriteria()} is null then return a default {@link StopCriteria}.
+ */
Review Comment:
Pls link with `Trainer` interface via `{@link ..}`
Pls add specific return value JavaDoc.
Pls set `null` as `{@code null}`, twice here.
##########
opennlp-tools/pom.xml:
##########
@@ -72,6 +72,13 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.assertj</groupId>
+ <artifactId>assertj-core</artifactId>
Review Comment:
@NishantShri4 Can we avoid AssertJ as extra dependency? I wonder, why assertions of JUnit 5.x are not sufficient?
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/LogLikelihoodThresholdBreached.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import opennlp.tools.util.TrainingParameters;
+
+import static opennlp.tools.ml.maxent.GISTrainer.LOG_LIKELIHOOD_THRESHOLD_DEFAULT;
+import static opennlp.tools.ml.maxent.GISTrainer.LOG_LIKELIHOOD_THRESHOLD_PARAM;
+
+/**
+ * A {@link StopCriteria} implementation to identify whether the
+ * difference between the log likelihood of current and previous iteration is under the defined threshold.
+ */
+public class LogLikelihoodThresholdBreached implements StopCriteria {
Review Comment:
Could require a change towards the idea to make use of generic <T extends Number> in the separate comment.
##########
opennlp-tools/src/main/java/opennlp/tools/ml/AbstractTrainer.java:
##########
@@ -108,4 +124,12 @@ protected void addToReport(String key, String value) {
reportMap.put(key, value);
}
+ /**
+ * Retrieves the {@link TrainingConfiguration} associated with a {@link AbstractTrainer}.
Review Comment:
Should read "with an...", extra "n"
##########
opennlp-tools/src/main/java/opennlp/tools/ml/TrainerFactory.java:
##########
@@ -180,6 +182,22 @@ public static <T> EventModelSequenceTrainer<T> getEventModelSequenceTrainer(
}
}
+ /**
+ * Works like {@link TrainerFactory#getEventTrainer(TrainingParameters, Map, TrainingConfiguration)}
+ * except that the {@link TrainingConfiguration} is initialized with {@link DefaultTrainingProgressMonitor}
+ * and a null {@link opennlp.tools.monitoring.StopCriteria}.
+ * If not provided, the actual {@link opennlp.tools.monitoring.StopCriteria}
+ * will be decided by the {@link EventTrainer} implementation.
+ *
Review Comment:
Pls add specific parameter JavaDoc.
Pls add specific return value JavaDoc.
##########
opennlp-tools/src/main/java/opennlp/tools/ml/maxent/GISTrainer.java:
##########
@@ -709,4 +727,25 @@ synchronized double getLoglikelihood() {
return loglikelihood;
}
}
+
+ /**
+ * Get the {@link StopCriteria} associated with this Trainer.
+ * @param trainingConfig - If {@link TrainingConfiguration} is null or
+ * {@link TrainingConfiguration#stopCriteria()} is null then return a default {@link StopCriteria}.
+ */
+ private StopCriteria getStopCriteria(TrainingConfiguration trainingConfig) {
+ return trainingConfig != null && trainingConfig.stopCriteria() != null
+ ? trainingConfig.stopCriteria() : new LogLikelihoodThresholdBreached(trainingParameters);
+ }
+
+ /**
+ * Get the {@link TrainingProgressMonitor} associated with this Trainer.
+ * @param trainingConfig If {@link TrainingConfiguration} is null or
+ * {@link TrainingConfiguration#progMon()} is null then return a default {@link TrainingProgressMonitor}.
+ */
Review Comment:
Pls link with `Trainer` interface via `{@link ..}`
Pls add specific return value JavaDoc.
Pls set `null` as `{@code null}`, twice here.
##########
opennlp-tools/src/main/java/opennlp/tools/ml/perceptron/PerceptronTrainer.java:
##########
@@ -432,4 +450,25 @@ private static boolean isPerfectSquare(int n) {
return root * root == n;
}
+ /**
+ * Get the {@link StopCriteria} associated with this Trainer.
+ * @param trainingConfig - If {@link TrainingConfiguration} is null or
+ * {@link TrainingConfiguration#stopCriteria()} is null then return a default {@link StopCriteria}.
+ */
Review Comment:
Pls link with `Trainer` interface via `{@link ..}`
Pls add specific return value JavaDoc.
Pls format `null` as `{@code null}`, twice here.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/DefaultTrainingProgressMonitor.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static opennlp.tools.monitoring.StopCriteria.FINISHED;
+
+/**
+ * An implementation of {@link TrainingProgressMonitor} which publishes model training progress to the chosen
Review Comment:
Better:
"The default implementation of ..."
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/DefaultTrainingProgressMonitor.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static opennlp.tools.monitoring.StopCriteria.FINISHED;
+
+/**
+ * An implementation of {@link TrainingProgressMonitor} which publishes model training progress to the chosen
+ * logging destination.
+ */
+public class DefaultTrainingProgressMonitor implements TrainingProgressMonitor {
+
+ private static final Logger logger = LoggerFactory.getLogger(DefaultTrainingProgressMonitor.class);
+
+ /**
+ * Keeps a track whether training was already finished because StopCriteria was met.
Review Comment:
Pls, use `{@link ...}` for `StopCriteria`
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
+ */
+
+public interface TrainingProgressMonitor {
+
+ /**
+ * Captures the Iteration progress.
+ *
+ * @param iteration The completed iteration number.
+ * @param numberCorrectEvents Number of correctly predicted events in this iteration.
+ * @param totalEvents Total count of events processed in this iteration.
+ * @param measure Measure used to quantify training success.
+ * @param measureValue measure value corresponding to the applicable {@link TrainingMeasure}.
+ */
+ void finishedIteration(int iteration, int numberCorrectEvents, int totalEvents,
+ TrainingMeasure measure, double measureValue);
+
+ /**
+ * Captures the Training completion progress.
+ *
+ * @param iterations Total number of iterations configured for training.
+ * @param stopCriteria Exit criteria for training.
Review Comment:
Pls {@link ...} here.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.function.Predicate;
+
+/**
+ * Stop-criteria for a {@link opennlp.tools.ml.model.AbstractModel} training .
Review Comment:
Pls, clear extra whitespace at end of sentence.
Better (?):
```
* Stop criteria for the training of models. If the predicate is met, the training is aborted.
*
* @see Predicate
* @see opennlp.tools.ml.model.AbstractModel
```
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/IterDeltaAccuracyUnderTolerance.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import opennlp.tools.ml.perceptron.PerceptronTrainer;
+import opennlp.tools.util.TrainingParameters;
+
+/**
+ * A {@link StopCriteria} implementation to identify whether the absolute
+ * difference between the training accuracy of current and previous iteration is under the defined tolerance.
+ */
+public class IterDeltaAccuracyUnderTolerance implements StopCriteria {
Review Comment:
Could require a change towards the idea to make use of generic `<T extends Number>` in the separate comment.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
Review Comment:
lower-cased for Training + Progress, as those a simple nouns here.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.function.Predicate;
+
+/**
+ * Stop-criteria for a {@link opennlp.tools.ml.model.AbstractModel} training .
+ */
+public interface StopCriteria extends Predicate<Double> {
+
+ String FINISHED = "Training Finished after completing %s Iterations successfully.";
+
+ /**
+ * @return - A detailed message when the stop-criteria is satisfied during model training.
Review Comment:
Pls avoid using "-" as a start for the return value JavaDoc. Check it for other occurrences.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
+ */
+
+public interface TrainingProgressMonitor {
+
+ /**
+ * Captures the Iteration progress.
+ *
+ * @param iteration The completed iteration number.
+ * @param numberCorrectEvents Number of correctly predicted events in this iteration.
+ * @param totalEvents Total count of events processed in this iteration.
+ * @param measure Measure used to quantify training success.
+ * @param measureValue measure value corresponding to the applicable {@link TrainingMeasure}.
+ */
+ void finishedIteration(int iteration, int numberCorrectEvents, int totalEvents,
+ TrainingMeasure measure, double measureValue);
+
+ /**
+ * Captures the Training completion progress.
Review Comment:
lower-cased for "Training" here.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
+ */
+
+public interface TrainingProgressMonitor {
+
+ /**
+ * Captures the Iteration progress.
+ *
+ * @param iteration The completed iteration number.
+ * @param numberCorrectEvents Number of correctly predicted events in this iteration.
+ * @param totalEvents Total count of events processed in this iteration.
+ * @param measure Measure used to quantify training success.
+ * @param measureValue measure value corresponding to the applicable {@link TrainingMeasure}.
+ */
+ void finishedIteration(int iteration, int numberCorrectEvents, int totalEvents,
+ TrainingMeasure measure, double measureValue);
+
+ /**
+ * Captures the Training completion progress.
+ *
+ * @param iterations Total number of iterations configured for training.
+ * @param stopCriteria Exit criteria for training.
+ */
+ void finishedTraining(int iterations, StopCriteria stopCriteria);
+
+ /**
+ * Checks whether the training has finished. *
Review Comment:
Pls remove extra "*" at the end of this line.
##########
opennlp-tools/src/test/java/opennlp/tools/ml/MockSequenceTrainer.java:
##########
@@ -34,5 +35,10 @@ public AbstractModel train(SequenceStream<Event> events) {
@Override
public void init(TrainingParameters trainParams, Map<String, String> reportMap) {
}
-
+
+ @Override
+ public void init(TrainingParameters trainParams, Map<String, String> reportMap
+ , TrainingConfiguration config) {
Review Comment:
pls move `,` to line above (40), looks odd.
##########
opennlp-tools/src/test/java/opennlp/tools/monitoring/IterDeltaAccuracyUnderToleranceTest.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.Map;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
+import opennlp.tools.util.TrainingParameters;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class IterDeltaAccuracyUnderToleranceTest {
+
+ private StopCriteria stopCriteria;
+
+ @BeforeEach
+ public void setup() {
+ stopCriteria = new IterDeltaAccuracyUnderTolerance(new TrainingParameters(Map.of("Tolerance",
+ .00002)));
+ }
+
+ @ParameterizedTest()
+ @CsvSource( {"0.01,false", "-0.01,false", "0.00001,true", "-0.00001,true"})
Review Comment:
That's a nice approach! 👍
##########
opennlp-tools/src/main/java/opennlp/tools/util/TrainingConfiguration.java:
##########
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.util;
+
+import opennlp.tools.monitoring.StopCriteria;
+import opennlp.tools.monitoring.TrainingProgressMonitor;
+
+/**
+ * Training Configuration used in {@link opennlp.tools.ml.model.AbstractModel} training.
Review Comment:
lower-cased for `Configuration` here.
##########
opennlp-tools/src/test/java/opennlp/tools/monitoring/DefaultTrainingProgressMonitorTest.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.slf4j.LoggerFactory;
+
+import opennlp.tools.util.TrainingParameters;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class DefaultTrainingProgressMonitorTest {
+
+ private static final String LOGGER_NAME = "opennlp";
+ private TrainingProgressMonitor progressMonitor;
Review Comment:
Pls reorganize fields and group static fields "en bloc" and non-static fields below.
##########
opennlp-tools/src/test/java/opennlp/tools/monitoring/IterDeltaAccuracyUnderToleranceTest.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.Map;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
+import opennlp.tools.util.TrainingParameters;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class IterDeltaAccuracyUnderToleranceTest {
+
+ private StopCriteria stopCriteria;
+
+ @BeforeEach
+ public void setup() {
+ stopCriteria = new IterDeltaAccuracyUnderTolerance(new TrainingParameters(Map.of("Tolerance",
+ .00002)));
+ }
+
+ @ParameterizedTest()
Review Comment:
Remove extra brackets here, not required when no arguments are specified.
##########
pom.xml:
##########
@@ -180,6 +180,7 @@
<!-- Dependency versions -->
<junit.version>5.12.1</junit.version>
<junit5-system-exit.version>2.0.2</junit5-system-exit.version>
+ <assertj-core.version>3.27.3</assertj-core.version>
Review Comment:
Avoid it, see comment(s) above.
##########
opennlp-tools/src/test/java/opennlp/tools/monitoring/LogLikelihoodThresholdBreachedTest.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.Map;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
+import opennlp.tools.util.TrainingParameters;
+
+import static opennlp.tools.ml.maxent.GISTrainer.LOG_LIKELIHOOD_THRESHOLD_PARAM;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+
+class LogLikelihoodThresholdBreachedTest {
+
+ private StopCriteria stopCriteria;
+
+ @BeforeEach
+ public void setup() {
+ stopCriteria = new LogLikelihoodThresholdBreached(
+ new TrainingParameters(Map.of(LOG_LIKELIHOOD_THRESHOLD_PARAM,5.)));
+ }
+
+ @ParameterizedTest()
Review Comment:
Remove extra brackets here, not required when no arguments are specified.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on PR #758:
URL: https://github.com/apache/opennlp/pull/758#issuecomment-2811048367
Thanks @mawiesne. This is done (rebase, squash and force push).
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2012761368
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/ConsoleTrainingProgressMonitor.java:
##########
@@ -0,0 +1,51 @@
+package opennlp.tools.monitoring;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+
+import static opennlp.tools.monitoring.StopCriteria.TRAINING_FINISHED_DEFAULT_MSG;
+
+/**
+ * Publishes ML model's Training progress to console.
+ */
+public class ConsoleTrainingProgressMonitor implements TrainingProgressMonitor {
+
+ private static final Logger logger = LoggerFactory.getLogger(ConsoleTrainingProgressMonitor.class);
+
+ /**
+ * Keeps a track whether training was already finished because StopCriteria was met.
+ */
+ volatile boolean isTrainingFinished;
+
+ private final List<String> progress = new LinkedList<>();
Review Comment:
Good point! Should we think about following?
1. Add an extractAndClear() method on TPM interface , which when invoked empties the underlying data-structure and returns the logged progress. The caller then decides what to do with the returned results?
2. finishedIteration() issues a warning when number of items logged reach a configured threshold e.g. 50k progress events logged.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2043081606
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/StopCriteria.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.function.Predicate;
+
+/**
+ * Stop-criteria for a {@link opennlp.tools.ml.model.AbstractModel} training .
+ */
+public interface StopCriteria extends Predicate<Double> {
Review Comment:
This can be made as extends _Predicate<Object>_
So that this common API is flexible to receive any object as input to verify testing Criteria? views pls.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2012774502
##########
opennlp-tools/src/main/java/opennlp/tools/ml/perceptron/PerceptronTrainer.java:
##########
@@ -257,15 +260,17 @@ public AbstractModel trainModel(int iterations, DataIndexer di, int cutoff, bool
logger.info("Computing model parameters...");
- MutableContext[] finalParameters = findParameters(iterations, useAverage);
+
+ //@TODO : Ideally an instance of TrainingProgressMonitor will be introduced via Trainer Factory.
+ MutableContext[] finalParameters = findParameters(iterations, useAverage, new ConsoleTrainingProgressMonitor());
Review Comment:
Thanks.
1. Shall we include a progressMonitor and stopCriteria in TrainingParameters?
2. Or else we create a new TrainingConfiguration entity ?
This embeds the two (i.e. TPM and StopCriteria) and is passed as an argument to an overloaded getEventTrainer(..)
method
`
public static EventTrainer getEventTrainer(
TrainingParameters trainParams, Map<String, String> reportMap, TrainingConfiguration config)
`
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2043689343
##########
opennlp-tools/src/test/java/opennlp/tools/monitoring/DefaultTrainingProgressMonitorTest.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.slf4j.LoggerFactory;
+
+import opennlp.tools.util.TrainingParameters;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class DefaultTrainingProgressMonitorTest {
+
+ private final String LOGGER_NAME = "opennlp";
+ private TrainingProgressMonitor progressMonitor;
+ private final ListAppender<ILoggingEvent> appender = new ListAppender<>();
Review Comment:
It seems `ListAppender` shows a different behavior on Ubunut/MacOs. The appender.list doesn't capture the logging events even after attaching this to the relevant LOGGER. Checking this. Should be able to fix in next push.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056021530
##########
opennlp-tools/src/main/java/opennlp/tools/ml/perceptron/PerceptronTrainer.java:
##########
@@ -432,4 +450,25 @@ private static boolean isPerfectSquare(int n) {
return root * root == n;
}
+ /**
+ * Get the {@link StopCriteria} associated with this Trainer.
+ * @param trainingConfig - If {@link TrainingConfiguration} is null or
+ * {@link TrainingConfiguration#stopCriteria()} is null then return a default {@link StopCriteria}.
+ */
+ private StopCriteria getStopCriteria(TrainingConfiguration trainingConfig) {
+ return trainingConfig != null && trainingConfig.stopCriteria() != null
+ ? trainingConfig.stopCriteria() : new IterDeltaAccuracyUnderTolerance(trainingParameters);
+ }
+
+ /**
+ * Get the {@link TrainingProgressMonitor} associated with this Trainer.
+ * @param trainingConfig - If {@link TrainingConfiguration} is null or
+ * {@link TrainingConfiguration#progMon()}is null then
+ * return the default {@link TrainingProgressMonitor}.
+ */
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056017037
##########
opennlp-tools/src/main/java/opennlp/tools/ml/perceptron/PerceptronTrainer.java:
##########
@@ -432,4 +450,25 @@ private static boolean isPerfectSquare(int n) {
return root * root == n;
}
+ /**
+ * Get the {@link StopCriteria} associated with this Trainer.
+ * @param trainingConfig - If {@link TrainingConfiguration} is null or
+ * {@link TrainingConfiguration#stopCriteria()} is null then return a default {@link StopCriteria}.
+ */
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on PR #758:
URL: https://github.com/apache/opennlp/pull/758#issuecomment-2829010316
> @NishantShri4 can you fill an ICLA for your contribution please?
>
> Details can be found here https://www.apache.org/licenses/contributor-agreements.html
>
> If you sign it, please add "OpenNLP" in the section "notify project". You don't need to add an Apache ID. Thanks!
Thanks @rzo1. This is done (signed ICLA is sent to :secretary@apache.org).
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056097239
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
+ */
+
+public interface TrainingProgressMonitor {
+
+ /**
+ * Captures the Iteration progress.
+ *
+ * @param iteration The completed iteration number.
+ * @param numberCorrectEvents Number of correctly predicted events in this iteration.
+ * @param totalEvents Total count of events processed in this iteration.
+ * @param measure Measure used to quantify training success.
Review Comment:
Thanks. Done.
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
+ */
+
+public interface TrainingProgressMonitor {
+
+ /**
+ * Captures the Iteration progress.
+ *
+ * @param iteration The completed iteration number.
+ * @param numberCorrectEvents Number of correctly predicted events in this iteration.
+ * @param totalEvents Total count of events processed in this iteration.
+ * @param measure Measure used to quantify training success.
+ * @param measureValue measure value corresponding to the applicable {@link TrainingMeasure}.
+ */
+ void finishedIteration(int iteration, int numberCorrectEvents, int totalEvents,
+ TrainingMeasure measure, double measureValue);
+
+ /**
+ * Captures the Training completion progress.
+ *
+ * @param iterations Total number of iterations configured for training.
+ * @param stopCriteria Exit criteria for training.
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056098106
##########
opennlp-tools/src/main/java/opennlp/tools/monitoring/TrainingProgressMonitor.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.tools.monitoring;
+
+/**
+ * An interface to capture Training Progress of a {@link opennlp.tools.ml.model.AbstractModel}.
+ */
+
+public interface TrainingProgressMonitor {
+
+ /**
+ * Captures the Iteration progress.
+ *
+ * @param iteration The completed iteration number.
+ * @param numberCorrectEvents Number of correctly predicted events in this iteration.
+ * @param totalEvents Total count of events processed in this iteration.
+ * @param measure Measure used to quantify training success.
+ * @param measureValue measure value corresponding to the applicable {@link TrainingMeasure}.
+ */
+ void finishedIteration(int iteration, int numberCorrectEvents, int totalEvents,
+ TrainingMeasure measure, double measureValue);
+
+ /**
+ * Captures the Training completion progress.
+ *
+ * @param iterations Total number of iterations configured for training.
+ * @param stopCriteria Exit criteria for training.
+ */
+ void finishedTraining(int iterations, StopCriteria stopCriteria);
+
+ /**
+ * Checks whether the training has finished. *
+ * @return A boolean value to identify whether the training has finished.
Review Comment:
Thanks. Done. Javadocs are revised.
##########
opennlp-tools/src/test/java/opennlp/tools/ml/MockSequenceTrainer.java:
##########
@@ -34,5 +35,10 @@ public AbstractModel train(SequenceStream<Event> events) {
@Override
public void init(TrainingParameters trainParams, Map<String, String> reportMap) {
}
-
+
+ @Override
+ public void init(TrainingParameters trainParams, Map<String, String> reportMap
+ , TrainingConfiguration config) {
Review Comment:
Thanks. Done.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2056082229
##########
opennlp-tools/pom.xml:
##########
@@ -72,6 +72,13 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.assertj</groupId>
+ <artifactId>assertj-core</artifactId>
Review Comment:
Thanks Done. (The reason to use Assertj was that it provided some simple methods to compare java collections. There are alternatives provided by junit5, which I have used now).
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on PR #758:
URL: https://github.com/apache/opennlp/pull/758#issuecomment-2828340560
@NishantShri4 can you fill an ICLA for your contribution please?
Details can be found here https://www.apache.org/licenses/contributor-agreements.html
If you sign it, please add "OpenNLP" in the section "notify project". Thanks!
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124: Maxent/Perceptron training should report progress back via an API (opennlp)
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne merged PR #758:
URL: https://github.com/apache/opennlp/pull/758
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-124 Maxent/Perceptron training should report progess back via an API (opennlp)
Posted by "NishantShri4 (via GitHub)" <gi...@apache.org>.
NishantShri4 commented on code in PR #758:
URL: https://github.com/apache/opennlp/pull/758#discussion_r2041237795
##########
opennlp-tools/src/main/java/opennlp/tools/ml/perceptron/PerceptronTrainer.java:
##########
@@ -257,15 +260,17 @@ public AbstractModel trainModel(int iterations, DataIndexer di, int cutoff, bool
logger.info("Computing model parameters...");
- MutableContext[] finalParameters = findParameters(iterations, useAverage);
+
+ //@TODO : Ideally an instance of TrainingProgressMonitor will be introduced via Trainer Factory.
+ MutableContext[] finalParameters = findParameters(iterations, useAverage, new ConsoleTrainingProgressMonitor());
Review Comment:
Thanks. Added a TrainingConfiguration Record object to initialize the trainer via TrainerFactory. The trainingConfiguration object embeds the TPM and StopCriteria.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org