[
https://issues.apache.org/jira/browse/OPENNLP-1159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16317524#comment-16317524
]
ASF GitHub Bot commented on OPENNLP-1159:
-----------------------------------------
kojisekig closed pull request #300: OPENNLP-1159: avoid letting users specify
AggregatedFeatureGeneratorF…
URL: https://github.com/apache/opennlp/pull/300
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/CachedFeatureGenerator.java
b/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/CachedFeatureGenerator.java
index d3f0705e5..f02a47e8f 100644
---
a/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/CachedFeatureGenerator.java
+++
b/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/CachedFeatureGenerator.java
@@ -37,11 +37,17 @@
private long numberOfCacheHits;
private long numberOfCacheMisses;
+ @Deprecated
public CachedFeatureGenerator(AdaptiveFeatureGenerator... generators) {
this.generator = new AggregatedFeatureGenerator(generators);
contextsCache = new Cache<>(100);
}
+ public CachedFeatureGenerator(AdaptiveFeatureGenerator generator) {
+ this.generator = generator;
+ contextsCache = new Cache<>(100);
+ }
+
public void createFeatures(List<String> features, String[] tokens, int index,
String[] previousOutcomes) {
@@ -103,4 +109,8 @@ public String toString() {
+ " misses=" + numberOfCacheMisses + " hit%" + (numberOfCacheHits > 0 ?
(double) numberOfCacheHits / (numberOfCacheMisses + numberOfCacheHits)
: 0);
}
+
+ public AdaptiveFeatureGenerator getCachedFeatureGenerator() {
+ return generator;
+ }
}
diff --git
a/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java
b/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java
index 26de2f9d2..867c1e043 100644
---
a/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java
+++
b/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java
@@ -52,10 +52,9 @@
* Creates a set of feature generators based on a provided XML descriptor.
*
* Example of an XML descriptor:
- *<p>
- * <generator
class="opennlp.tools.util.featuregen.AggregatedFeatureGeneratorFactory">
+ * <p>
+ * <featureGenerators name="namefind">
* <generator
class="opennlp.tools.util.featuregen.CachedFeatureGeneratorFactory">
- * <generator
class="opennlp.tools.util.featuregen.AggregatedFeatureGeneratorFactory">
* <generator
class="opennlp.tools.util.featuregen.WindowFeatureGeneratorFactory">
* <int name="prevLength">2</int>
* <int name="nextLength">2</int>
@@ -73,9 +72,8 @@
* <bool name="begin">true</bool>
* <bool name="end">false</bool>
* </generator>
- * </generator>
* </generator>
- * </generator>
+ * </featureGenerators>
* </p>
*
* Each XML element is mapped to a {@link
GeneratorFactory.XmlFeatureGeneratorFactory} which
@@ -140,7 +138,7 @@ final void init(Element element,
FeatureGeneratorResourceProvider resourceManage
throws InvalidFormatException {
this.generatorElement = element;
this.resourceManager = resourceManager;
- int generators = 0;
+ List<AdaptiveFeatureGenerator> generators = new ArrayList<>();
NodeList childNodes = generatorElement.getChildNodes();
for (int i = 0; i < childNodes.getLength(); i++) {
Node childNode = childNodes.item(i);
@@ -148,8 +146,9 @@ final void init(Element element,
FeatureGeneratorResourceProvider resourceManage
Element elem = (Element)childNode;
String type = elem.getTagName();
if (type.equals("generator")) {
- String key = "generator#" + Integer.toString(generators++);
+ String key = "generator#" + Integer.toString(generators.size());
AdaptiveFeatureGenerator afg = buildGenerator(elem,
resourceManager);
+ generators.add(afg);
if (afg != null)
args.put(key, afg);
}
@@ -185,6 +184,13 @@ final void init(Element element,
FeatureGeneratorResourceProvider resourceManage
}
}
}
+
+ if (generators.size() > 1) {
+ AdaptiveFeatureGenerator aggregatedFeatureGenerator =
+ new AggregatedFeatureGenerator(generators.toArray(
+ new AdaptiveFeatureGenerator[generators.size()]));
+ args.put("generator#0", aggregatedFeatureGenerator);
+ }
}
public int getInt(String name) throws InvalidFormatException {
@@ -447,14 +453,27 @@ static AdaptiveFeatureGenerator createGenerator(Element
generatorElement,
// check it is new format?
if (elementName.equals("featureGenerators")) {
- Element firstElem = getFirstChild(generatorElement);
- if (firstElem != null) {
- if (firstElem.getTagName().equals("generator")) {
- return buildGenerator(firstElem, resourceManager);
+
+ List<AdaptiveFeatureGenerator> generators = new ArrayList<>();
+ NodeList childNodes = generatorElement.getChildNodes();
+ for (int i = 0; i < childNodes.getLength(); i++) {
+ Node childNode = childNodes.item(i);
+ if (childNode instanceof Element) {
+ Element elem = (Element)childNode;
+ String type = elem.getTagName();
+ if (type.equals("generator")) {
+ generators.add(buildGenerator(elem, resourceManager));
+ }
+ else
+ throw new InvalidFormatException("Unexpected element: " +
elementName);
}
- else
- throw new InvalidFormatException("Unexpected element: " +
elementName);
}
+
+ if (generators.size() == 1)
+ return generators.get(0);
+ else if (generators.size() > 1)
+ return new AggregatedFeatureGenerator(generators.toArray(
+ new AdaptiveFeatureGenerator[generators.size()]));
else
throw new InvalidFormatException("featureGenerators must have one or
more generators");
}
diff --git
a/opennlp-tools/src/main/resources/opennlp/tools/namefind/ner-default-features.xml
b/opennlp-tools/src/main/resources/opennlp/tools/namefind/ner-default-features.xml
index 58288e1b0..32887cf78 100644
---
a/opennlp-tools/src/main/resources/opennlp/tools/namefind/ner-default-features.xml
+++
b/opennlp-tools/src/main/resources/opennlp/tools/namefind/ner-default-features.xml
@@ -19,9 +19,7 @@
<!-- Default name finder feature generator configuration -->
<featureGenerators name="nameFinder">
- <generator
class="opennlp.tools.util.featuregen.AggregatedFeatureGeneratorFactory">
<generator
class="opennlp.tools.util.featuregen.CachedFeatureGeneratorFactory">
- <generator
class="opennlp.tools.util.featuregen.AggregatedFeatureGeneratorFactory">
<generator
class="opennlp.tools.util.featuregen.WindowFeatureGeneratorFactory">
<int name="prevLength">2</int>
<int name="nextLength">2</int>
@@ -39,7 +37,5 @@
<bool name="begin">true</bool>
<bool name="end">false</bool>
</generator>
- </generator>
</generator>
- </generator>
</featureGenerators>
diff --git
a/opennlp-tools/src/main/resources/opennlp/tools/postag/pos-default-features.xml
b/opennlp-tools/src/main/resources/opennlp/tools/postag/pos-default-features.xml
index 466cba785..c1be8ee6e 100644
---
a/opennlp-tools/src/main/resources/opennlp/tools/postag/pos-default-features.xml
+++
b/opennlp-tools/src/main/resources/opennlp/tools/postag/pos-default-features.xml
@@ -19,9 +19,7 @@
<!-- Default pos tagger feature generator configuration -->
<featureGenerators name="posTagger">
- <generator
class="opennlp.tools.util.featuregen.AggregatedFeatureGeneratorFactory">
<generator
class="opennlp.tools.util.featuregen.CachedFeatureGeneratorFactory">
- <generator
class="opennlp.tools.util.featuregen.AggregatedFeatureGeneratorFactory">
<generator
class="opennlp.tools.util.featuregen.DefinitionFeatureGeneratorFactory"/>
<generator
class="opennlp.tools.util.featuregen.SuffixFeatureGeneratorFactory"/>
<generator
class="opennlp.tools.util.featuregen.PrefixFeatureGeneratorFactory"/>
@@ -40,7 +38,5 @@
</generator>
<generator
class="opennlp.tools.util.featuregen.TokenClassFeatureGeneratorFactory"/>
<generator
class="opennlp.tools.util.featuregen.PosTaggerFeatureGeneratorFactory"/>
- </generator>
</generator>
- </generator>
</featureGenerators>
diff --git
a/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/GeneratorFactoryClassicFormatTest.java
b/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/GeneratorFactoryClassicFormatTest.java
index cdf1404b8..ffc6b97f2 100644
---
a/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/GeneratorFactoryClassicFormatTest.java
+++
b/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/GeneratorFactoryClassicFormatTest.java
@@ -55,7 +55,7 @@ public void testCreationWithTokenClassFeatureGenerator()
throws Exception {
@Test
public void testCreationWihtSimpleDescriptor() throws Exception {
InputStream generatorDescriptorIn = getClass().getResourceAsStream(
- "/opennlp/tools/util/featuregen/TestFeatureGeneratorConfig.xml");
+
"/opennlp/tools/util/featuregen/TestFeatureGeneratorConfig_classic.xml");
// If this fails the generator descriptor could not be found
// at the expected location
diff --git
a/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/GeneratorFactoryTest.java
b/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/GeneratorFactoryTest.java
index f97401171..35a58bbe9 100644
---
a/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/GeneratorFactoryTest.java
+++
b/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/GeneratorFactoryTest.java
@@ -172,4 +172,76 @@ public void testParameters() throws Exception {
Assert.assertTrue(featureGenerator.bp);
Assert.assertEquals("HELLO", featureGenerator.sp);
}
+
+ @Test
+ public void testNotAutomaticallyInsertAggregatedFeatureGenerator() throws
Exception {
+ InputStream generatorDescriptorIn = getClass().getResourceAsStream(
+
"/opennlp/tools/util/featuregen/TestNotAutomaticallyInsertAggregatedFeatureGenerator.xml");
+
+ // If this fails the generator descriptor could not be found
+ // at the expected location
+ Assert.assertNotNull(generatorDescriptorIn);
+
+ AdaptiveFeatureGenerator featureGenerator =
GeneratorFactory.create(generatorDescriptorIn, null);
+ Assert.assertTrue(featureGenerator instanceof
OutcomePriorFeatureGenerator);
+ }
+
+ @Test
+ public void testAutomaticallyInsertAggregatedFeatureGenerator() throws
Exception {
+ InputStream generatorDescriptorIn = getClass().getResourceAsStream(
+
"/opennlp/tools/util/featuregen/TestAutomaticallyInsertAggregatedFeatureGenerator.xml");
+
+ // If this fails the generator descriptor could not be found
+ // at the expected location
+ Assert.assertNotNull(generatorDescriptorIn);
+
+ AdaptiveFeatureGenerator featureGenerator =
GeneratorFactory.create(generatorDescriptorIn, null);
+ Assert.assertTrue(featureGenerator instanceof AggregatedFeatureGenerator);
+
+ AggregatedFeatureGenerator aggregatedFeatureGenerator =
(AggregatedFeatureGenerator)featureGenerator;
+ Assert.assertEquals(3, aggregatedFeatureGenerator.getGenerators().size());
+ for (AdaptiveFeatureGenerator afg:
aggregatedFeatureGenerator.getGenerators()) {
+ Assert.assertTrue(afg instanceof OutcomePriorFeatureGenerator);
+ }
+ }
+
+ @Test
+ public void testNotAutomaticallyInsertAggregatedFeatureGeneratorChild()
throws Exception {
+ InputStream generatorDescriptorIn = getClass().getResourceAsStream(
+
"/opennlp/tools/util/featuregen/TestNotAutomaticallyInsertAggregatedFeatureGeneratorChild.xml");
+
+ // If this fails the generator descriptor could not be found
+ // at the expected location
+ Assert.assertNotNull(generatorDescriptorIn);
+
+ AdaptiveFeatureGenerator featureGenerator =
GeneratorFactory.create(generatorDescriptorIn, null);
+ Assert.assertTrue(featureGenerator instanceof CachedFeatureGenerator);
+
+ CachedFeatureGenerator cachedFeatureGenerator =
(CachedFeatureGenerator)featureGenerator;
+ Assert.assertTrue(cachedFeatureGenerator.getCachedFeatureGenerator()
+ instanceof OutcomePriorFeatureGenerator);
+ }
+
+ @Test
+ public void testAutomaticallyInsertAggregatedFeatureGeneratorChildren()
throws Exception {
+ InputStream generatorDescriptorIn = getClass().getResourceAsStream(
+
"/opennlp/tools/util/featuregen/TestAutomaticallyInsertAggregatedFeatureGeneratorChildren.xml");
+
+ // If this fails the generator descriptor could not be found
+ // at the expected location
+ Assert.assertNotNull(generatorDescriptorIn);
+
+ AdaptiveFeatureGenerator featureGenerator =
GeneratorFactory.create(generatorDescriptorIn, null);
+ Assert.assertTrue(featureGenerator instanceof CachedFeatureGenerator);
+
+ CachedFeatureGenerator cachedFeatureGenerator =
(CachedFeatureGenerator)featureGenerator;
+ AdaptiveFeatureGenerator afg =
cachedFeatureGenerator.getCachedFeatureGenerator();
+ Assert.assertTrue(afg instanceof AggregatedFeatureGenerator);
+
+ AggregatedFeatureGenerator aggregatedFeatureGenerator =
(AggregatedFeatureGenerator)afg;
+ Assert.assertEquals(3, aggregatedFeatureGenerator.getGenerators().size());
+ for (AdaptiveFeatureGenerator afgen:
aggregatedFeatureGenerator.getGenerators()) {
+ Assert.assertTrue(afgen instanceof OutcomePriorFeatureGenerator);
+ }
+ }
}
diff --git
a/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestAutomaticallyInsertAggregatedFeatureGenerator.xml
b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestAutomaticallyInsertAggregatedFeatureGenerator.xml
new file mode 100644
index 000000000..82e2a743c
--- /dev/null
+++
b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestAutomaticallyInsertAggregatedFeatureGenerator.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ 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.
+-->
+
+<featureGenerators name="test">
+ <generator
class="opennlp.tools.util.featuregen.DefinitionFeatureGeneratorFactory"/>
+ <generator
class="opennlp.tools.util.featuregen.DefinitionFeatureGeneratorFactory"/>
+ <generator
class="opennlp.tools.util.featuregen.DefinitionFeatureGeneratorFactory"/>
+</featureGenerators>
diff --git
a/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestAutomaticallyInsertAggregatedFeatureGeneratorChildren.xml
b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestAutomaticallyInsertAggregatedFeatureGeneratorChildren.xml
new file mode 100644
index 000000000..7dbed59a0
--- /dev/null
+++
b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestAutomaticallyInsertAggregatedFeatureGeneratorChildren.xml
@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ 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.
+-->
+
+<featureGenerators name="test">
+ <generator
class="opennlp.tools.util.featuregen.CachedFeatureGeneratorFactory">
+ <generator
class="opennlp.tools.util.featuregen.DefinitionFeatureGeneratorFactory"/>
+ <generator
class="opennlp.tools.util.featuregen.DefinitionFeatureGeneratorFactory"/>
+ <generator
class="opennlp.tools.util.featuregen.DefinitionFeatureGeneratorFactory"/>
+ </generator>
+</featureGenerators>
diff --git
a/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestFeatureGeneratorConfig.xml
b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestFeatureGeneratorConfig.xml
index 6518948a2..8ebb56f91 100644
---
a/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestFeatureGeneratorConfig.xml
+++
b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestFeatureGeneratorConfig.xml
@@ -19,7 +19,8 @@
under the License.
-->
-<generators>
- <definition/>
-</generators>
-
+<featureGenerators name="test">
+ <generator
class="opennlp.tools.util.featuregen.AggregatedFeatureGeneratorFactory">
+ <generator
class="opennlp.tools.util.featuregen.DefinitionFeatureGeneratorFactory"/>
+ </generator>
+</featureGenerators>
diff --git
a/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestNotAutomaticallyInsertAggregatedFeatureGenerator.xml
b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestNotAutomaticallyInsertAggregatedFeatureGenerator.xml
new file mode 100644
index 000000000..7a15b9984
--- /dev/null
+++
b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestNotAutomaticallyInsertAggregatedFeatureGenerator.xml
@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ 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.
+-->
+
+<featureGenerators name="test">
+ <generator
class="opennlp.tools.util.featuregen.DefinitionFeatureGeneratorFactory"/>
+</featureGenerators>
diff --git
a/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestNotAutomaticallyInsertAggregatedFeatureGeneratorChild.xml
b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestNotAutomaticallyInsertAggregatedFeatureGeneratorChild.xml
new file mode 100644
index 000000000..ed7f2f698
--- /dev/null
+++
b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestNotAutomaticallyInsertAggregatedFeatureGeneratorChild.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ 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.
+-->
+
+<featureGenerators name="test">
+ <generator
class="opennlp.tools.util.featuregen.CachedFeatureGeneratorFactory">
+ <generator
class="opennlp.tools.util.featuregen.DefinitionFeatureGeneratorFactory"/>
+ </generator>
+</featureGenerators>
diff --git
a/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestTokenClassFeatureGeneratorConfig.xml
b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestTokenClassFeatureGeneratorConfig.xml
index e384a7ef1..63b7b7c6c 100644
---
a/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestTokenClassFeatureGeneratorConfig.xml
+++
b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestTokenClassFeatureGeneratorConfig.xml
@@ -19,6 +19,10 @@
under the License.
-->
-<generators>
- <tokenclass wordAndClass="true"/>
-</generators>
\ No newline at end of file
+<featureGenerators name="test">
+ <generator
class="opennlp.tools.util.featuregen.AggregatedFeatureGeneratorFactory">
+ <generator
class="opennlp.tools.util.featuregen.TokenClassFeatureGeneratorFactory">
+ <bool name="wordAndClass">true</bool>
+ </generator>
+ </generator>
+</featureGenerators>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> avoid letting users specify AggregatedFeatureGeneratorFactory in XML config
> ---------------------------------------------------------------------------
>
> Key: OPENNLP-1159
> URL: https://issues.apache.org/jira/browse/OPENNLP-1159
> Project: OpenNLP
> Issue Type: Improvement
> Components: Formats, Name Finder
> Affects Versions: 1.8.3
> Reporter: Koji Sekiguchi
> Assignee: Koji Sekiguchi
> Priority: Minor
>
> When I'm working on OPENNLP-1154, I think we should do it for better use.
> I'd like to implement this as an independent ticket from OPENNLP-1154 to make
> patch easy to read.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
