search

LearnLib / learnlib

A free, open-source Java library for automata learning algorithms
https://learnlib.de
Apache License 2.0
206 stars 53 forks source link

feat: lsharp #107

Closed tiferrei closed 10 months ago

tiferrei commented 11 months ago

A very literal translation of gitlab.science.ru.nl/sws/lsharp @ 8526fc3a88fa18b0c408d867385bcc9a29a302a1

mtf90 commented 11 months ago

Just for your sanity: You can run the code-analysis locally as well with the -Pcode-analysis maven profile :laughing:.

tiferrei commented 11 months ago

Just for your sanity: You can run the code-analysis locally as well with the -Pcode-analysis maven profile 😆.

Unfortunately my LearnLib maven seems incredibly unreliable! I sometimes have to compile things multiple times to compile fully, and even stuff like mvn pmd:check does not list all violations locally!

mtf90 commented 11 months ago

Have you tried doing a mvn clean install -Pcode-analysis? It may be that the individual plugin invocations (such as mvn pmd:check) do not pick up all the configurations. The install phase with the -Pcode-analysis profile should cover everything (and is exactly what the CI pipline does as well).

tiferrei commented 11 months ago

Indeed, however for example, this is the error from the first run of mvn clean install -Pcode-analysis:

[INFO] <<< spotbugs:4.7.3.6:check (default) < :spotbugs @ learnlib-adt <<<
[INFO]
[INFO]
[INFO] --- spotbugs:4.7.3.6:check (default) @ learnlib-adt ---
[INFO] BugInstance size is 6
[INFO] Error size is 0
[INFO] Total bugs: 6
[ERROR] Medium: Unused field: de.learnlib.algorithm.adt.learner.ADTLearnerBuilder.adtExtender [de.learnlib.algorithm.adt.learner.ADTLearnerBuilder] In ADTLearnerBuilder.java UUF_UNUSED_FIELD
[ERROR] Medium: Unused field: de.learnlib.algorithm.adt.learner.ADTLearnerBuilder.alphabet [de.learnlib.algorithm.adt.learner.ADTLearnerBuilder] In ADTLearnerBuilder.java UUF_UNUSED_FIELD
[ERROR] Medium: Unused field: de.learnlib.algorithm.adt.learner.ADTLearnerBuilder.leafSplitter [de.learnlib.algorithm.adt.learner.ADTLearnerBuilder] In ADTLearnerBuilder.java UUF_UNUSED_FIELD
[ERROR] Medium: Unused field: de.learnlib.algorithm.adt.learner.ADTLearnerBuilder.oracle [de.learnlib.algorithm.adt.learner.ADTLearnerBuilder] In ADTLearnerBuilder.java UUF_UNUSED_FIELD
[ERROR] Medium: Unused field: de.learnlib.algorithm.adt.learner.ADTLearnerBuilder.subtreeReplacer [de.learnlib.algorithm.adt.learner.ADTLearnerBuilder] In ADTLearnerBuilder.java UUF_UNUSED_FIELD
[ERROR] Medium: Unused field: de.learnlib.algorithm.adt.learner.ADTLearnerBuilder.useObservationTree [de.learnlib.algorithm.adt.learner.ADTLearnerBuilder] In ADTLearnerBuilder.java UUF_UNUSED_FIELD

And I have not modified those modules at all, and the CI indeed dos not report those issues.

This seems to happen with all kinds of different commands. For example, in compile I've had missing Classes, etc. Which just get solved by running the command again, same for unit tests. It seems every time I run maven it flips a die 😄

mtf90 commented 11 months ago

Hm, this indeed looks weird, because even without the proper configuration files for spotbugs, I don't receive these kind of violations. Which Maven version, Java (both version and distribution), and OS do you use?

I would first look into this issue, because there will be a lot more violations (I briefly skimmed over the PR and a lot of files are missing the license header for example) and it will be a pain to work them off via CI ping-pong.

tiferrei commented 11 months ago

Yes indeed, here are my versions:

Apache Maven 3.9.5 (57804ffe001d7215b5e7bcb531cf83df38f93546)
Maven home: /opt/homebrew/Cellar/maven/3.9.5/libexec
Java version: 21.0.1, vendor: Homebrew, runtime: /opt/homebrew/Cellar/openjdk/21.0.1/libexec/openjdk.jdk/Contents/Home
Default locale: en_GB, platform encoding: UTF-8
OS name: "mac os x", version: "14.1.1", arch: "aarch64", family: "mac"
mtf90 commented 11 months ago

Hm, even with JDK21+Maven3.9.5 I cannot reproduce these errors. However, I use Linux. I can ask some of my colleagues who use a Mac on monday whether they can reproduce it.

tiferrei commented 11 months ago

It's very weird. For example, this is a failure on running mvn install in AutomataLib (a pre-requisite), in Docker ubuntu:23.04 with openjdk-21-dk and maven installed with apt:

mvn --version:

Apache Maven 3.8.7
Maven home: /usr/share/maven
Java version: 21-ea, vendor: Private Build, runtime: /usr/lib/jvm/java-21-openjdk-arm64
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "6.5.10-orbstack-00110-gbcfe04c86d2f", arch: "aarch64", family: "unix"

mvn install:

[...]

[INFO] --- maven-surefire-plugin:3.1.2:test (default-test) @ automata-modelchecking-ltsmin ---
[INFO] Using auto detected provider org.apache.maven.surefire.testng.TestNGProvider
[INFO]
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running TestSuite
SLF4J: No SLF4J providers were found.
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See https://www.slf4j.org/codes.html#noProviders for further details.
[ERROR] Tests run: 101, Failures: 11, Errors: 0, Skipped: 88, Time elapsed: 0.435 s <<< FAILURE! -- in TestSuite
[ERROR] net.automatalib.modelchecker.ltsmin.LTSminLTLParserTest.testIOFormulae[[] (letter == "1" -> X letter == "2"), false](1) -- Time elapsed: 0.003 s <<< FAILURE!
java.lang.Error:
Unresolved compilation problems:
    InternalLTSminLTLParser cannot be resolved to a type
    InternalLTSminLTLParser cannot be resolved to a type
    ParseException cannot be resolved to a type
    No exception of type Object can be thrown; an exception type must be a subclass of Throwable
    TokenMgrError cannot be resolved to a type
    The constructor IllegalArgumentException(String, Object) is undefined

    at net.automatalib.modelchecker.ltsmin.LTSminLTLParser.requireValidIOFormula(LTSminLTLParser.java:109)
    at net.automatalib.modelchecker.ltsmin.LTSminLTLParser.isValidIOFormula(LTSminLTLParser.java:132)
    at net.automatalib.modelchecker.ltsmin.LTSminLTLParserTest.testIOFormulae(LTSminLTLParserTest.java:66)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:578)
    at org.testng.internal.invokers.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:136)
    at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:658)
    at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:219)
    at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:50)
    at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:923)
    at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:192)
    at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
    at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:128)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
    at org.testng.TestRunner.privateRun(TestRunner.java:808)
    at org.testng.TestRunner.run(TestRunner.java:603)
    at org.testng.SuiteRunner.runTest(SuiteRunner.java:429)
    at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:423)
    at org.testng.SuiteRunner.privateRun(SuiteRunner.java:383)
    at org.testng.SuiteRunner.run(SuiteRunner.java:326)
    at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
    at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:95)
    at org.testng.TestNG.runSuitesSequentially(TestNG.java:1249)
    at org.testng.TestNG.runSuitesLocally(TestNG.java:1169)
    at org.testng.TestNG.runSuites(TestNG.java:1092)
    at org.testng.TestNG.run(TestNG.java:1060)
    at org.apache.maven.surefire.testng.TestNGExecutor.run(TestNGExecutor.java:155)
    at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeMulti(TestNGDirectoryTestSuite.java:169)
    at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.execute(TestNGDirectoryTestSuite.java:88)
    at org.apache.maven.surefire.testng.TestNGProvider.invoke(TestNGProvider.java:137)
    at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
    at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
    at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
    at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)

[...]

The only common variable here seems to be that this is still the ARM image. I then tried with Docker on an AMD64 image (using native emulation). Same docker image and packages:

mvn --version:

Apache Maven 3.8.7
Maven home: /usr/share/maven
Java version: 21-ea, vendor: Private Build, runtime: /usr/lib/jvm/java-21-openjdk-amd64
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "6.5.10-orbstack-00110-gbcfe04c86d2f", arch: "amd64", family: "unix"

And mvn install completed with no problems, passing all tests. However it then failed on LearnLib, again in a module that I have not modified, with some missing class errors:

[...]

[INFO] --- maven-compiler-plugin:3.10.1:testCompile (default-testCompile) @ learnlib-equivalence-oracles ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 19 source files to /code/learnlib/oracles/equivalence-oracles/target/test-classes
[INFO]
[INFO] --- maven-surefire-plugin:3.1.2:test (default-test) @ learnlib-equivalence-oracles ---
[INFO] Using auto detected provider org.apache.maven.surefire.testng.TestNGProvider
[INFO]
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running TestSuite
SLF4J: No SLF4J providers were found.
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See https://www.slf4j.org/codes.html#noProviders for further details.
OpenJDK 64-Bit Server VM warning: Sharing is only supported for boot loader classes because bootstrap classpath has been appended
[ERROR] Tests run: 43, Failures: 13, Errors: 0, Skipped: 15, Time elapsed: 5.979 s <<< FAILURE! -- in TestSuite
[ERROR] de.learnlib.oracle.equivalence.DisproveFirstOracleTest.setUp -- Time elapsed: 3.888 s <<< FAILURE!
java.lang.NoClassDefFoundError: de/learnlib/oracle/equivalence/DisproveFirstOracle
    at de.learnlib.oracle.equivalence.DisproveFirstOracleTest.setUp(DisproveFirstOracleTest.java:64)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:578)
    at org.testng.internal.invokers.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:136)
    at org.testng.internal.invokers.MethodInvocationHelper.invokeMethodConsideringTimeout(MethodInvocationHelper.java:65)
    at org.testng.internal.invokers.ConfigInvoker.invokeConfigurationMethod(ConfigInvoker.java:381)
    at org.testng.internal.invokers.ConfigInvoker.invokeConfigurations(ConfigInvoker.java:319)
    at org.testng.internal.invokers.TestInvoker.runConfigMethods(TestInvoker.java:781)
    at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:574)
    at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:219)
    at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:50)
    at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:923)
    at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:192)
    at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
    at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:128)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
    at org.testng.TestRunner.privateRun(TestRunner.java:808)
    at org.testng.TestRunner.run(TestRunner.java:603)
    at org.testng.SuiteRunner.runTest(SuiteRunner.java:429)
    at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:423)
    at org.testng.SuiteRunner.privateRun(SuiteRunner.java:383)
    at org.testng.SuiteRunner.run(SuiteRunner.java:326)
    at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
    at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:95)
    at org.testng.TestNG.runSuitesSequentially(TestNG.java:1249)
    at org.testng.TestNG.runSuitesLocally(TestNG.java:1169)
    at org.testng.TestNG.runSuites(TestNG.java:1092)
    at org.testng.TestNG.run(TestNG.java:1060)
    at org.apache.maven.surefire.testng.TestNGExecutor.run(TestNGExecutor.java:155)
    at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeMulti(TestNGDirectoryTestSuite.java:169)
    at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.execute(TestNGDirectoryTestSuite.java:88)
    at org.apache.maven.surefire.testng.TestNGProvider.invoke(TestNGProvider.java:137)
    at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
    at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
    at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
    at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)
Caused by: java.lang.ClassNotFoundException: de.learnlib.oracle.equivalence.DisproveFirstOracle
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:526)
    ... 36 more

[...]
mtf90 commented 11 months ago

Hm, 21-ea doesn't sound like the most stable version. Could you try the ga release or maybe even Java 17 (LTS as well)?

tiferrei commented 11 months ago

Ah yes, good point. I did this with OpenJDK-17-jdk, other variables as before:

mvn install:

[...]

[INFO] --- maven-surefire-plugin:3.1.2:test (default-test) @ learnlib-membership-oracles ---
[INFO] Using auto detected provider org.apache.maven.surefire.testng.TestNGProvider
[INFO]
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running TestSuite
SLF4J: No SLF4J providers were found.
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See https://www.slf4j.org/codes.html#noProviders for further details.
[ERROR] Tests run: 9, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 2.154 s <<< FAILURE! -- in TestSuite
[ERROR] de.learnlib.oracle.membership.SimulatorOmegaOracleTest.testDFASimulatorOmegaOracle -- Time elapsed: 0.012 s <<< FAILURE!
java.lang.Error:
Unresolved compilation problems:
    ExamplePaulAndMary cannot be resolved
    ExamplePaulAndMary cannot be resolved to a variable
    ExamplePaulAndMary cannot be resolved to a variable
    ExamplePaulAndMary cannot be resolved to a variable
    ExamplePaulAndMary cannot be resolved to a variable
    ExamplePaulAndMary cannot be resolved to a variable
    ExamplePaulAndMary cannot be resolved to a variable
    ExamplePaulAndMary cannot be resolved to a variable

    at de.learnlib.oracle.membership.SimulatorOmegaOracleTest.testDFASimulatorOmegaOracle(SimulatorOmegaOracleTest.java:38)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at org.testng.internal.invokers.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:136)
    at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:658)
    at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:219)
    at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:50)
    at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:923)
    at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:192)
    at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
    at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:128)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
    at org.testng.TestRunner.privateRun(TestRunner.java:808)
    at org.testng.TestRunner.run(TestRunner.java:603)
    at org.testng.SuiteRunner.runTest(SuiteRunner.java:429)
    at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:423)
    at org.testng.SuiteRunner.privateRun(SuiteRunner.java:383)
    at org.testng.SuiteRunner.run(SuiteRunner.java:326)
    at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
    at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:95)
    at org.testng.TestNG.runSuitesSequentially(TestNG.java:1249)
    at org.testng.TestNG.runSuitesLocally(TestNG.java:1169)
    at org.testng.TestNG.runSuites(TestNG.java:1092)
    at org.testng.TestNG.run(TestNG.java:1060)
    at org.apache.maven.surefire.testng.TestNGExecutor.run(TestNGExecutor.java:155)
    at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeMulti(TestNGDirectoryTestSuite.java:169)
    at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.execute(TestNGDirectoryTestSuite.java:88)
    at org.apache.maven.surefire.testng.TestNGProvider.invoke(TestNGProvider.java:137)
    at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
    at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
    at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
    at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)

[INFO]
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR]   SimulatorOmegaOracleTest.testDFASimulatorOmegaOracle:38  Unresolved compilation problems:
    ExamplePaulAndMary cannot be resolved
    ExamplePaulAndMary cannot be resolved to a variable
    ExamplePaulAndMary cannot be resolved to a variable
    ExamplePaulAndMary cannot be resolved to a variable
    ExamplePaulAndMary cannot be resolved to a variable
    ExamplePaulAndMary cannot be resolved to a variable
    ExamplePaulAndMary cannot be resolved to a variable
    ExamplePaulAndMary cannot be resolved to a variable

[INFO]
[ERROR] Tests run: 9, Failures: 1, Errors: 0, Skipped: 0

[...]

So again some weird failures, as you can see with some unrelated classes too.

mvn --version:

Apache Maven 3.8.7
Maven home: /usr/share/maven
Java version: 17.0.8.1, vendor: Private Build, runtime: /usr/lib/jvm/java-17-openjdk-amd64
Default locale: en_US, platform encoding: ANSI_X3.4-1968
OS name: "linux", version: "6.5.10-orbstack-00110-gbcfe04c86d2f", arch: "amd64", family: "unix"
mtf90 commented 11 months ago

Can you try a mvn clean install -DskipTests? It's kind of suspicous that normal compilation seems to work, but as soon as some reflection/introspection happens, things break. Also, do you experience similar problems elsewhere (e.g., when running your IDE or compiling other projects)? I'm trying to isolate whether this is a Java issue, a Maven issue, or a AutomataLib/Learnlib (build process) issue.

tiferrei commented 11 months ago

Unfortunately it also seems to give me issues:

[...]

[INFO] --------< de.learnlib.testsupport:learnlib-learner-it-support >---------
[INFO] Building LearnLib :: Test Support :: Learner IT Support 0.18.0-SNAPSHOT [22/58]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ learnlib-learner-it-support ---
[INFO] Deleting /code/learnlib/test-support/learner-it-support/target
[INFO]
[INFO] --- maven-resources-plugin:2.6:resources (default-resources) @ learnlib-learner-it-support ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory /code/learnlib/test-support/learner-it-support/src/main/resources
[INFO]
[INFO] --- maven-compiler-plugin:3.10.1:compile (default-compile) @ learnlib-learner-it-support ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 37 source files to /code/learnlib/test-support/learner-it-support/target/classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] /code/learnlib/test-support/learner-it-support/src/main/java/de/learnlib/testsupport/it/learner/AbstractMealyLearnerIT.java:[75,29] cannot access SimulatorEQOracle
  class file for SimulatorEQOracle not found
[INFO] 1 error
[INFO] -------------------------------------------------------------

[...]

When I use my IDE I never really have issues, however I rarely compile through it. I mostly use it for autocomplete and editing, and then switch to cli maven when I want to run tests.

mtf90 commented 11 months ago

I have checked with two of my colleagues:

On an Intel-based MacBook

Apache Maven 3.8.6 (84538c9988a25aec085021c365c560670ad80f63)
Maven home: /usr/local/Cellar/maven/3.8.6/libexec
Java version: 21.0.1, vendor: Oracle Corporation, runtime: /Library/Java/JavaVirtualMachines/jdk-21.jdk/Contents/Home
Default locale: de_DE, platform encoding: UTF-8
OS name: "mac os x", version: "13.0", arch: "x86_64", family: "mac"

everything worked as expected (both regular compilation and the code-analysis profile)

On an M1-based MacBook, both a Rosetta-emulated JVM

Apache Maven 3.9.5 (57804ffe001d7215b5e7bcb531cf83df38f93546)
Maven home: /opt/homebrew/Cellar/maven/3.9.5/libexec
Java version: 11.0.18, vendor: Amazon.com Inc., runtime: /Library/Java/JavaVirtualMachines/amazon-corretto-11.jdk/Contents/Home
Default locale: de_DE, platform encoding: UTF-8
OS name: "mac os x", version: "14.1.1", arch: "x86_64", family: "mac"

as well as a native JVM

Apache Maven 3.9.5 (57804ffe001d7215b5e7bcb531cf83df38f93546)
Maven home: /opt/homebrew/Cellar/maven/3.9.5/libexec
Java version: 21.0.1, vendor: Homebrew, runtime: /opt/homebrew/Cellar/openjdk/21.0.1/libexec/openjdk.jdk/Contents/Home
Default locale: de_DE, platform encoding: UTF-8
OS name: "mac os x", version: "14.1.1", arch: "aarch64", family: "mac"

caused no problems in both cases either.

So I am afraid that this looks like a problem on your end. Unfortunately, I don't use a Mac myself so I cannot really provide any further guidance. Maybe you can find some more troubleshooting information online?

tiferrei commented 11 months ago

Very weird indeed. I honestly think I'll stick to the CI output, I have sank too much time trying to fix the maven issue and it seems to only happen when I work on learnlib

mtf90 commented 11 months ago

I think you can allow maintainers to push on your PR as well. If you would like, I can have a look at the verifications later and (hopefully) fix them in one go.

tiferrei commented 11 months ago

That would be wonderful, thank you! I think I should be pretty close now, but I admit I am not familiar with Learnlib or java in general conventions. If there's anything further you'd like changed I am totally happy for it to be done.

mtf90 commented 11 months ago

Allright, I'll have a look at it later today (I think, you'll get notified about any commits). Then we can talk about the actuall contents of the PR :laughing:.

tiferrei commented 11 months ago

Great, I will stop committing as of this moment, until further notice, to avoid conflicts.

mtf90 commented 11 months ago

You're right, there were only a few things left that needed change. Since I don't have push-access, see the diff below. The refactoring may be a little bit invasive, but I didn't see any reason why the Helpers class needed to be Serializable.

Can you give a short run-down of what the algorithm does (or maybe reference the paper where it is presented)? Does the ADS stuff stand for adaptive distinguishing sequences? There already exists some code in the AutomataLib that we may be able to re-use.

diff --git a/algorithms/active/lsharp/src/main/java/de/learnlib/algorithm/lsharp/ads/Helpers.java b/algorithms/active/lsharp/src/main/java/de/learnlib/algorithm/lsharp/ads/Helpers.java
index 48bd3b945..5acdd8ddc 100644
--- a/algorithms/active/lsharp/src/main/java/de/learnlib/algorithm/lsharp/ads/Helpers.java
+++ b/algorithms/active/lsharp/src/main/java/de/learnlib/algorithm/lsharp/ads/Helpers.java
@@ -16,24 +16,26 @@

 package de.learnlib.algorithm.lsharp.ads;

-import java.io.Serializable;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.PriorityQueue;
+import java.util.Set;

 import net.automatalib.common.util.Pair;

-// We specify the implementations to guarantee they are Serializable.
-@SuppressWarnings("PMD.LooseCoupling")
-public class Helpers<I> implements Serializable {
-    public final HashSet<Integer> nodesInTree = new HashSet<>();
-    public final PriorityQueue<Pair<Integer, Integer>> partition = new PriorityQueue<>(new PairComparator());
-    public final HashSet<Integer> dependent = new HashSet<>();
-    public final PriorityQueue<Pair<Integer, Integer>> dependentPrioQueue = new PriorityQueue<>(new PairComparator());
-    public final HashMap<Integer, HashSet<Pair<Integer, I>>> transitionsTo = new HashMap<>();
-    public final HashSet<Integer> analysedIndices = new HashSet<>();
-    public final HashMap<Integer, BestNode<I>> bestNode = new HashMap<>();
+public class Helpers<I> {
+
+    public final Set<Integer> nodesInTree = new HashSet<>();
+    public final PriorityQueue<Pair<Integer, Integer>> partition =
+            new PriorityQueue<>(Comparator.comparing(Pair::getSecond));
+    public final Set<Integer> dependent = new HashSet<>();
+    public final PriorityQueue<Pair<Integer, Integer>> dependentPrioQueue =
+            new PriorityQueue<>(Comparator.comparing(Pair::getSecond));
+    public final Map<Integer, HashSet<Pair<Integer, I>>> transitionsTo = new HashMap<>();
+    public final Set<Integer> analysedIndices = new HashSet<>();
+    public final Map<Integer, BestNode<I>> bestNode = new HashMap<>();

     public void addTransitionFromToVia(Integer src, Integer dst, I via) {
         this.transitionsTo.computeIfAbsent(dst, k -> new HashSet<>()).add(Pair.of(src, via));
@@ -43,16 +45,4 @@ public class Helpers<I> implements Serializable {
         return this.bestNode.get(r).score;
     }

-    private class PairComparator implements Comparator<Pair<Integer, Integer>>, Serializable {
-        @Override
-        public int compare(Pair<Integer, Integer> p1, Pair<Integer, Integer> p2) {
-            if (p1.getSecond() < p2.getSecond()) {
-                return 1;
-            } else if (p1.getSecond() > p2.getSecond()) {
-                return -1;
-            }
-            return 0;
-        }
-    }
-
 }
tiferrei commented 11 months ago

Sure, this is a very direct translation of the original L# implementation first described in 2022.

I don't have any take on it, I just needed to run some benchmarks for a paper, and the original implementation is in Rust while all our infra uses Learnlib. So I ported it over a few years ago and thought it would be good to send it upstream. :)

mtf90 commented 11 months ago

Thanks for the link, I'll have a read when the time allows for it. Given that it is a straight adoption of the rust prototype, the code could probably benefit from some refactoring to better integrate with the existing functionality. I'm happy to look into it, but it will take some time. From the sound of yours, it doesn't seem to be high priority either, right?

tiferrei commented 11 months ago

That would be great. This is not pressing for me at all, I just found our fork diverging more and more and wanted to start syncing things up :)

mtf90 commented 11 months ago

Dear @tiferrei, I have read the paper but I don't know how to feel about it.

Content-wise, many of the concepts can already be found in existing algorithms.

Code-wise, the code could probably benefit from some refactoring.

I don't know. At the moment, the "return on investment" doesn't really feel great. You have mentioned that you implemented the algorithm for some benchmarks of your own. Have you seen any scenarios where L# posed a significant improvement or where it provided a feature that existing algorithms do not?

And something completely off-topic: We are currently collecting projects that work with LearnLib in one way or another. You mentioned that your group at UCL uses LearnLib and I can see from your fork that you have been quite busy. Would you like to link the work/tool of your group as well?

tiferrei commented 11 months ago

Hi @mtf90, thanks for the review.

Indeed, personally I don't claim any pros or cons with L#. Initially we made no mention of it at all on our papers, however reviewers were adamant (for reasons beyond my understanding) that we compare to L# in our benchmarks. Content-wise I agree with your points, and really just added the benchmarks and comparison to finally get the paper published.

Code-wise, I agree too. My aim for this code was not for it be super performant (time complexity wise), however I did want to maintain equivalence in terms of query complexity with the original implementation. Thus why I opted for such a literal translation -- I get to compare to "the algorithm" without having to use the Rust implementation, as most of our work is in LearnLib.

I was going to just throw away the code, however given that the reviewers seemed to demonstrate such a big interest, it wouldn't surprise me if other people wanted to compare to it too. I don't blame you for not seeing the ROI in investing in improving the code -- neither would I. Perhaps we can leave the PR is a somewhat "dormant" state, in case someone looking for a learn lib implementation of L# can find it, and potentially use / improve it?

As for the off-topic: Yes! I do work with LearnLib quite a lot. My PhD is in developing automata learning for lightweight formal methods, and although the work is not always directly a tool, we at least implement proof-of-concepts, usually in LearnLib as it's such a well rounded framework.

As far as tools go, our most used one is PROGNOSIS: Paper, GitHub, which uses LearnLib for the learners. We also have more experimental papers on Tree-Based Adaptive Model Learning and C3AL: Conflict-Aware Active Automata Learning that we implement in LearnLib. It would be great to have these linked, if you'd like to!

mtf90 commented 10 months ago

Dear @tiferrei, I'm glad that you see things similar to me and aren't too disappointed about my reluctance to merge the code. Also, your work shouldn't be in vain and I agree with you that the implementation might by useful to others. However, I don't think that a (even dormant) PR is a good place to keep it. With ongoing development, the code bases drift apart further and further and the applicability of this PR becomes less and less.

Given that you have migrated the current version to the latest LearnLib release, I think a good compromise would be to host the L# code (without LearnLib) in a separate repository at your UCL group and simply reference the relevant LearnLib/AutomataLib parts via the deployed Maven dependencies. This would also allow you to adapt, e.g., the license headers or Maven GAV-coordinates to your UCL group for proper attribution. Depending on whether you want to support future development/improvements, you could then decide whether you want to keep it as a regular repository or transform it into a public archive.

I added your links to the overview. Also, I just realized that we have neighboring papers in Frits' Festschrift. Small world we live in :).

tiferrei commented 10 months ago

I think that's a good solution! We will probably keep it as a public archive, and slim it down to just depend on AutomataLib, thanks for the suggestion.

PS: A small world indeed!