You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@opennlp.apache.org by "mawiesne (via GitHub)" <gi...@apache.org> on 2023/02/04 09:54:22 UTC
[GitHub] [opennlp-sandbox] mawiesne opened a new pull request, #81: Update sandbox component 'opennlp-dl' to be compatible with latest opennlp-tools release
mawiesne opened a new pull request, #81:
URL: https://github.com/apache/opennlp-sandbox/pull/81
- adjusts opennlp-tools to 2.1.0
- adjusts Java language level to 11
- updates several dependencies to more up-to-date versions to mitigate several CVEs
- removes `nd4j-jblas` dep from 'opennlp-similarity' as was only required for a transitive Spring dependency :-/
- adjusts code to changes in various dependencies
- ignores existing, non-working JUnit tests
- removes unused imports
- adds 'opennlp-dl' module to parent pom
--
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
[GitHub] [opennlp-sandbox] kinow commented on pull request #81: Update sandbox component 'opennlp-dl' to be compatible with latest opennlp-tools release
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #81:
URL: https://github.com/apache/opennlp-sandbox/pull/81#issuecomment-1418204839
> @kinow Updated PR to manage platform-specific DL4J deps via Maven profiles. This helps locally and to keep GH Actions Cache small(er) than now... FYI: @rzo1
Do you still have the test errors on Mac? i.e. any chance the errors were due to some issue with the previous dependencies?
--
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
[GitHub] [opennlp-sandbox] mawiesne commented on pull request #81: Update sandbox component 'opennlp-dl' to be compatible with latest opennlp-tools release
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on PR #81:
URL: https://github.com/apache/opennlp-sandbox/pull/81#issuecomment-1418204486
@kinow Updated PR to manage platform-specific DL4J deps via Maven profiles. This helps locally and to keep GH Actions Cache small(er) than now... FYI: @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
[GitHub] [opennlp-sandbox] mawiesne commented on pull request #81: Update sandbox component 'opennlp-dl' to be compatible with latest opennlp-tools release
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on PR #81:
URL: https://github.com/apache/opennlp-sandbox/pull/81#issuecomment-1418205941
> Do you still have the test errors on Mac? i.e. any chance the errors were due to some issue with the previous dependencies?
Yes, errors still present. I just managed the dependencies that were present before. Test are still ignored - seems code/environment related. I have no clue, atm.
--
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
[GitHub] [opennlp-sandbox] kinow commented on a diff in pull request #81: Update sandbox component 'opennlp-dl' to be compatible with latest opennlp-tools release
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on code in PR #81:
URL: https://github.com/apache/opennlp-sandbox/pull/81#discussion_r1096732664
##########
opennlp-dl/src/test/java/opennlp/tools/dl/RNNTest.java:
##########
@@ -68,6 +70,11 @@ public static Collection<Object[]> data() {
}
@Test
+ @Ignore
+ // TODO check why this fails with:
+ // java.lang.IllegalStateException: Can't transpose array with rank < 2: array shape [62]
+ // ...
+ // on MacOS (only?)
Review Comment:
:eyes: :+1:
--
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
[GitHub] [opennlp-sandbox] kinow commented on pull request #81: Update sandbox component 'opennlp-dl' to be compatible with latest opennlp-tools release
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #81:
URL: https://github.com/apache/opennlp-sandbox/pull/81#issuecomment-1418110154
>@kinow @rzo1 I found ways to modernize the code so it compiles and tests execute against latest DL4J / ND4J backend. However, on my MacOS machine it fails at runtime; see comment in RNNTest or StackedRNNTest.
I think macos/windows build errors on sandbox components are OK, as long as we have a TODO or Jira issue to track it down, and it's not something really big (e.g. most of the tests are failing for some unknown reason on macos).
Thanks for also fixing typos, javadoc, improving the code quality, and adding the TODO's :+1:
--
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
[GitHub] [opennlp-sandbox] mawiesne commented on pull request #81: Update sandbox component 'opennlp-dl' to be compatible with latest opennlp-tools release
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on PR #81:
URL: https://github.com/apache/opennlp-sandbox/pull/81#issuecomment-1418050381
> It shows tests are broken (thus ignored) with latest DL4J release... I tried my best to modernize it, however, the latest `nd4j` (1.0.0-M2.1) lacks `org.nd4j.linalg.api.ops.impl.transforms.OldSoftMax` and I have neither found: (i) documentation, (ii) changelogs nor (iii) deprecation infos or hints to correctly modernize code that relies on it.
>
> If you can find/provide infos on `OldSoftMax` and what happened with it, please feel free to share here.
@kinow @rzo1 I found ways to modernize the code so it compiles and the tests execute against ND4J backend. However, on my MacOS machine it fails at runtime; see comment in `RNNTest` or `StackedRNNTest`.
--
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
[GitHub] [opennlp-sandbox] mawiesne commented on pull request #81: Update sandbox component 'opennlp-dl' to be compatible with latest opennlp-tools release
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on PR #81:
URL: https://github.com/apache/opennlp-sandbox/pull/81#issuecomment-1418864888
> solves the init exception
Fixed for all platforms with latest 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
[GitHub] [opennlp-sandbox] rzo1 commented on pull request #81: Update sandbox component 'opennlp-dl' to be compatible with latest opennlp-tools release
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on PR #81:
URL: https://github.com/apache/opennlp-sandbox/pull/81#issuecomment-1418860473
Adding
```java
<dependency>
<groupId>org.nd4j</groupId>
<artifactId>nd4j-native</artifactId>
<version>${nd4j.native.version}</version>
</dependency>
```
solves the init exception for on Linux. Now I get the same exception as @mawiesne
--
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
[GitHub] [opennlp-sandbox] kinow commented on pull request #81: Update sandbox component 'opennlp-dl' to be compatible with latest opennlp-tools release
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #81:
URL: https://github.com/apache/opennlp-sandbox/pull/81#issuecomment-1418087944
Great @mawiesne ! Just working on a PR for Jena, then will come back and finish reviewing this one. 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
[GitHub] [opennlp-sandbox] rzo1 commented on a diff in pull request #81: Update sandbox component 'opennlp-dl' to be compatible with latest opennlp-tools release
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #81:
URL: https://github.com/apache/opennlp-sandbox/pull/81#discussion_r1097099607
##########
opennlp-dl/src/main/java/opennlp/tools/dl/NeuralDocCatModel.java:
##########
@@ -9,16 +33,10 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.io.*;
-import java.util.*;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipInputStream;
-import java.util.zip.ZipOutputStream;
-
/**
* This class is a wrapper for DL4J's {@link MultiLayerNetwork}, and {@link GlobalVectors}
* that provides features to serialize and deserialize necessary data to a zip file.
- *
+ * <p>
* This cane be used by a Neural Trainer tool to serialize the network and a predictor tool to restore the same network
Review Comment:
`can`
##########
opennlp-dl/src/main/java/opennlp/tools/dl/NeuralDocCatModel.java:
##########
@@ -129,16 +146,17 @@ public int getMaxSeqLen() {
}
/**
- * Zips the current state of the model and writes it stream
- * @param stream stream to write
- * @throws IOException
+ * Zips the current state of the model and writes it stream.
Review Comment:
`into the given stream` ?
--
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
[GitHub] [opennlp-sandbox] mawiesne commented on pull request #81: Update sandbox component 'opennlp-dl' to be compatible with latest opennlp-tools release
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on PR #81:
URL: https://github.com/apache/opennlp-sandbox/pull/81#issuecomment-1416714392
@kinow @rzo1 This PR migrates `opennlp-dl` (sandbox) to latest tools release.
It shows tests are broken (thus ignored) with latest DL4J release... I tried my best to modernize it, however, the latest `nd4j` (1.0.0-M2.1) lacks `OldSoftMax` and I have neither found: (i) documentation, (ii) changelogs nor (iii) deprecation infos or hints to correctly modernize it.
If you can find/provide infos on `OldSoftMax` and what happened with it, please feel free to share here.
--
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
[GitHub] [opennlp-sandbox] rzo1 merged pull request #81: Update sandbox component 'opennlp-dl' to be compatible with latest opennlp-tools release
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 merged PR #81:
URL: https://github.com/apache/opennlp-sandbox/pull/81
--
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