Lucene - Core
  1. Lucene - Core
  2. LUCENE-2934

Alternative depth-based DOT layout ordering in FST's Utils

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-ALPHA
    • Component/s: core/other
    • Labels:
      None

      Description

      Utils.toDot() dumps GraphViz's DOT file, but it can be quite difficult to read. This patch provides an alternative layout that is probably a little bit easier on the eyes (well, as far as larger FSTs can be

      1. LUCENE-2934.patch
        10 kB
        Dawid Weiss
      2. graph-after.png
        118 kB
        Dawid Weiss

        Activity

        Hide
        Dawid Weiss added a comment -

        Committed to trunk.

        Show
        Dawid Weiss added a comment - Committed to trunk.
        Hide
        Dawid Weiss added a comment -

        I know the principles of collective code ownership, but still – that function was/is primarly so that you can do the debugging of FST code, so I didn't want to interfere. As for the DOT: layered representation may cause arcs to overlap and be virtually impossible to read, so both reps. are useful, it just depends on the automaton. I'll correct it to Writer and commit in.

        Show
        Dawid Weiss added a comment - I know the principles of collective code ownership, but still – that function was/is primarly so that you can do the debugging of FST code, so I didn't want to interfere. As for the DOT: layered representation may cause arcs to overlap and be virtually impossible to read, so both reps. are useful, it just depends on the automaton. I'll correct it to Writer and commit in.
        Hide
        Michael McCandless added a comment -

        I admit I didn't want to take the liberty of changing too much since this is Mike's sacred Utils place

        Nobody owns any code in Apache There are no names attached. We all should always feel free to fix anything! Especially, newly created code should get lots of attention from others; the more the better.

        This better dot formatting looks awesome!! Keeping both options open seems good? (Though, I lack intuition on when the non-layered (old way) option would be better... maybe we default to the new way?).

        Show
        Michael McCandless added a comment - I admit I didn't want to take the liberty of changing too much since this is Mike's sacred Utils place Nobody owns any code in Apache There are no names attached. We all should always feel free to fix anything! Especially, newly created code should get lots of attention from others; the more the better. This better dot formatting looks awesome!! Keeping both options open seems good? (Though, I lack intuition on when the non-layered (old way) option would be better... maybe we default to the new way?).
        Hide
        Dawid Weiss added a comment -

        I admit I didn't want to take the liberty of changing too much since this is Mike's sacred Utils place I agree that these methods should accept a character stream (I'd even say a Writer because we don't need the wrapping PrintWriter that much), so if Mike doesn't mind, I'll change it to be so.

        Show
        Dawid Weiss added a comment - I admit I didn't want to take the liberty of changing too much since this is Mike's sacred Utils place I agree that these methods should accept a character stream (I'd even say a Writer because we don't need the wrapping PrintWriter that much), so if Mike doesn't mind, I'll change it to be so.
        Hide
        Dawid Weiss added a comment -

        Point taken. I would say both ways are useful, but for different aspects. I wish we had default parameter values, but since we don't wouldn't it be sensible to keep both methods (toDot(FST, PrintStream) with sane-defaults)?

        Show
        Dawid Weiss added a comment - Point taken. I would say both ways are useful, but for different aspects. I wish we had default parameter values, but since we don't wouldn't it be sensible to keep both methods (toDot(FST, PrintStream) with sane-defaults)?
        Hide
        Uwe Schindler added a comment -

        Additionally, PrintStream is a legacy class from Java 1.0. If DOT is specified to be US-ASCII, it should use a PrintWriter that is instantiated using US-ASCII as charset (PrintStream uses platform default, and that may be Big5 or other ugly things). In my opinion, the method should take a PrintWriter and a possible File/OutputStream method should be hardcoded to US-ASCII.

        Show
        Uwe Schindler added a comment - Additionally, PrintStream is a legacy class from Java 1.0. If DOT is specified to be US-ASCII, it should use a PrintWriter that is instantiated using US-ASCII as charset (PrintStream uses platform default, and that may be Big5 or other ugly things). In my opinion, the method should take a PrintWriter and a possible File/OutputStream method should be hardcoded to US-ASCII.
        Hide
        Uwe Schindler added a comment -

        Backwards compatibility is in my opinion not needed, the FST classes are new in 4.0.

        Show
        Uwe Schindler added a comment - Backwards compatibility is in my opinion not needed, the FST classes are new in 4.0.
        Hide
        Dawid Weiss added a comment -

        Patch for this. If there are no objections, I'll commit it in.

        Show
        Dawid Weiss added a comment - Patch for this. If there are no objections, I'll commit it in.
        Hide
        Dawid Weiss added a comment -

        Visual difference between the layered and free ordered dot file.

        Show
        Dawid Weiss added a comment - Visual difference between the layered and free ordered dot file.

          People

          • Assignee:
            Dawid Weiss
            Reporter:
            Dawid Weiss
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development