Lucene - Core
  1. Lucene - Core
  2. LUCENE-3768

Typo in some of the .alg files: ForcMerge instead of ForceMerge

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0-ALPHA
    • Fix Version/s: 3.6, 4.0-ALPHA
    • Component/s: modules/benchmark
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Some of the alg files have a typo and have ForcMerge when they should have ForceMerge. This causes following exception to display when those are used:

           [java] java.lang.Exception: Error: cannot understand algorithm!
           [java] 	at org.apache.lucene.benchmark.byTask.Benchmark.<init>(Benchmark.java:63)
           [java] 	at org.apache.lucene.benchmark.byTask.Benchmark.exec(Benchmark.java:109)
           [java] 	at org.apache.lucene.benchmark.byTask.Benchmark.main(Benchmark.java:84)
           [java] Caused by: java.lang.ClassNotFoundException: ForcMerge not found in packages [org.apache.lucene.benchmark.byTask.tasks]
           [java] 	at org.apache.lucene.benchmark.byTask.utils.Algorithm.taskClass(Algorithm.java:283)
           [java] 	at org.apache.lucene.benchmark.byTask.utils.Algorithm.<init>(Algorithm.java:73)
           [java] 	at org.apache.lucene.benchmark.byTask.Benchmark.<init>(Benchmark.java:61)
           [java] 	... 2 more
      

      Caused by: java.lang.ClassNotFoundException: ForcMerge not found in packages [org.apache.lucene.benchmark.byTask.tasks]

      1. LUCENE-3768.patch
        8 kB
        Sami Siren
      2. LUCENE-3768.patch
        12 kB
        Sami Siren
      3. LUCENE-3768.patch
        15 kB
        Robert Muir

        Activity

        Hide
        Sami Siren added a comment -

        a fix for this issue

        Show
        Sami Siren added a comment - a fix for this issue
        Hide
        Michael McCandless added a comment -

        Ugh, how silly of me... maybe we need a unit test that does simple validation (verifies the algs are parseable) of all the example algs under conf...

        Thanks Sami! +1

        Show
        Michael McCandless added a comment - Ugh, how silly of me... maybe we need a unit test that does simple validation (verifies the algs are parseable) of all the example algs under conf... Thanks Sami! +1
        Hide
        Sami Siren added a comment -

        A new patch that adds a junit test that parses through the examples.

        I am not sure if there is a better way to access the conf dir from a test.

        Show
        Sami Siren added a comment - A new patch that adds a junit test that parses through the examples. I am not sure if there is a better way to access the conf dir from a test.
        Hide
        Michael McCandless added a comment -

        Ooh, test case looks great; thanks!

        I don't know of a cleaner/official way to "get" the conf dir from a test... so I think your way is good

        Show
        Michael McCandless added a comment - Ooh, test case looks great; thanks! I don't know of a cleaner/official way to "get" the conf dir from a test... so I think your way is good
        Hide
        Robert Muir added a comment -

        I don't know of a cleaner/official way to "get" the conf dir from a test... so I think your way is good

        I agree, lets add the test.

        Here is my suggestion for an 'official' way.
        Just like we copy stuff in resources/ to the build directory,
        before benchmark runs tests it can copy conf/ to the build directory, so its in the classpath.

        For IDEs like eclipse, they would just add conf/ to the IDE classpath and tests would work there, too.

        Show
        Robert Muir added a comment - I don't know of a cleaner/official way to "get" the conf dir from a test... so I think your way is good I agree, lets add the test. Here is my suggestion for an 'official' way. Just like we copy stuff in resources/ to the build directory, before benchmark runs tests it can copy conf/ to the build directory, so its in the classpath. For IDEs like eclipse, they would just add conf/ to the IDE classpath and tests would work there, too.
        Hide
        Robert Muir added a comment -

        here's a patch... working from ant and eclipse.

        still a little hacky, but works.

        Show
        Robert Muir added a comment - here's a patch... working from ant and eclipse. still a little hacky, but works.
        Hide
        Sami Siren added a comment -

        The latest patch seems to work fine here.

        Show
        Sami Siren added a comment - The latest patch seems to work fine here.
        Hide
        Robert Muir added a comment -

        Thanks Sami, (especially for the test, too) I'll commit this.

        Show
        Robert Muir added a comment - Thanks Sami, (especially for the test, too) I'll commit this.
        Hide
        Robert Muir added a comment -

        Thanks Sami, upon backporting to 3.x, your test found some other bug in benchmark... its temporarily disabled in 3.x until we figure out whats going on... I'll open a separate issue.

        Show
        Robert Muir added a comment - Thanks Sami, upon backporting to 3.x, your test found some other bug in benchmark... its temporarily disabled in 3.x until we figure out whats going on... I'll open a separate issue.
        Hide
        Robert Muir added a comment -

        I opened LUCENE-3790 for the followup bug

        Show
        Robert Muir added a comment - I opened LUCENE-3790 for the followup bug

          People

          • Assignee:
            Robert Muir
            Reporter:
            Sami Siren
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development