Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Major
    • Resolution: Implemented
    • Affects Version/s: 1.3.0
    • Fix Version/s: 1.3.0
    • Component/s: Gelly
    • Labels:
      None

      Description

      Create AlgorithmResult and AnalyticResult interfaces for library algorithms to implement. This flattens algorithm results to a single tuple.

      Also create interfaces for UnaryResult, BinaryResult, and TertiaryResult implementing methods to access the 0th, 1st, and 2nd vertices.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user greghogan opened a pull request:

          https://github.com/apache/flink/pull/3434

          FLINK-5909 [gelly] Interface for GraphAlgorithm results

          Create AlgorithmResult and AnalyticResult interfaces for library algorithms to implement. This flattens algorithm results to a single tuple.

          Also create interfaces for UnaryResult, BinaryResult, and TertiaryResult implementing methods to access the 0th, 1st, and 2nd vertices.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/greghogan/flink 5909_interface_for_graphalgorithm_results

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/flink/pull/3434.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #3434


          commit 70f6780fad4a1231ce99dd03027b17a95e108dbc
          Author: Greg Hogan <code@greghogan.com>
          Date: 2017-02-28T18:10:20Z

          FLINK-5909 [gelly] Interface for GraphAlgorithm results

          Create AlgorithmResult and AnalyticResult interfaces for library
          algorithms to implement. This flattens algorithm results to a single
          tuple.

          Also create interfaces for UnaryResult, BinaryResult, and TertiaryResult
          implementing methods to access the 0th, 1st, and 2nd vertices.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/3434 FLINK-5909 [gelly] Interface for GraphAlgorithm results Create AlgorithmResult and AnalyticResult interfaces for library algorithms to implement. This flattens algorithm results to a single tuple. Also create interfaces for UnaryResult, BinaryResult, and TertiaryResult implementing methods to access the 0th, 1st, and 2nd vertices. You can merge this pull request into a Git repository by running: $ git pull https://github.com/greghogan/flink 5909_interface_for_graphalgorithm_results Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3434.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3434 commit 70f6780fad4a1231ce99dd03027b17a95e108dbc Author: Greg Hogan <code@greghogan.com> Date: 2017-02-28T18:10:20Z FLINK-5909 [gelly] Interface for GraphAlgorithm results Create AlgorithmResult and AnalyticResult interfaces for library algorithms to implement. This flattens algorithm results to a single tuple. Also create interfaces for UnaryResult, BinaryResult, and TertiaryResult implementing methods to access the 0th, 1st, and 2nd vertices.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vasia commented on the issue:

          https://github.com/apache/flink/pull/3434

          Hi @greghogan, thank you for the PR.

          I didn't spot anything that needs fixing, but I'm wondering what's the motivation to add these interfaces. I see how `toVerboseString()` is useful, but not really why `AnalyticResult` is needed. Also, why introduce `UnaryResult`, `BinaryResult`, and `TertiaryResult` instead of simply using tuple types?

          I also see that this PR contains no changes to the docs and that the current 1.3-SNAPSHOT docs already reflect the changes of this PR. What am I missing here?

          Show
          githubbot ASF GitHub Bot added a comment - Github user vasia commented on the issue: https://github.com/apache/flink/pull/3434 Hi @greghogan, thank you for the PR. I didn't spot anything that needs fixing, but I'm wondering what's the motivation to add these interfaces. I see how `toVerboseString()` is useful, but not really why `AnalyticResult` is needed. Also, why introduce `UnaryResult`, `BinaryResult`, and `TertiaryResult` instead of simply using tuple types? I also see that this PR contains no changes to the docs and that the current 1.3-SNAPSHOT docs already reflect the changes of this PR. What am I missing here?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user greghogan commented on the issue:

          https://github.com/apache/flink/pull/3434

          Thanks for the review @vasia. The Unary/Binary/TertiaryResult interfaces will allow follow-on algorithms to work with POJOs or non-conforming Tuples (where we don't assume source and target vertices are fields 0 and 1). A common AnalyticResult interface is helpful with drivers that store both directed and undirected results.

          The improvements to the library methods docs do reflect an expectation of this PR.

          Show
          githubbot ASF GitHub Bot added a comment - Github user greghogan commented on the issue: https://github.com/apache/flink/pull/3434 Thanks for the review @vasia. The Unary/Binary/TertiaryResult interfaces will allow follow-on algorithms to work with POJOs or non-conforming Tuples (where we don't assume source and target vertices are fields 0 and 1). A common AnalyticResult interface is helpful with drivers that store both directed and undirected results. The improvements to the library methods docs do reflect an expectation of this PR.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vasia commented on the issue:

          https://github.com/apache/flink/pull/3434

          Thanks for the clarification @greghogan. +1 from me.

          Show
          githubbot ASF GitHub Bot added a comment - Github user vasia commented on the issue: https://github.com/apache/flink/pull/3434 Thanks for the clarification @greghogan. +1 from me.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user greghogan commented on the issue:

          https://github.com/apache/flink/pull/3434

          @vasia, thinking again on `AnalyticResult`, as an empty interface I could simply replace with `GraphAnalytic<K, VV, EV, ? extends AnalyticResult>` with `GraphAnalytic<K, VV, EV, ? extends Object>`, for example [here](https://github.com/apache/flink/pull/3294/files#diff-fe7c1ff55b1b1e4cd1ee7809933607b5R60). Thoughts?

          Show
          githubbot ASF GitHub Bot added a comment - Github user greghogan commented on the issue: https://github.com/apache/flink/pull/3434 @vasia, thinking again on `AnalyticResult`, as an empty interface I could simply replace with `GraphAnalytic<K, VV, EV, ? extends AnalyticResult>` with `GraphAnalytic<K, VV, EV, ? extends Object>`, for example [here] ( https://github.com/apache/flink/pull/3294/files#diff-fe7c1ff55b1b1e4cd1ee7809933607b5R60 ). Thoughts?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vasia commented on the issue:

          https://github.com/apache/flink/pull/3434

          Does `AnalyticResult` also need a method like `toVerboseString()`? Could we replace both with a e.g. `Result` type?

          Show
          githubbot ASF GitHub Bot added a comment - Github user vasia commented on the issue: https://github.com/apache/flink/pull/3434 Does `AnalyticResult` also need a method like `toVerboseString()`? Could we replace both with a e.g. `Result` type?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user greghogan commented on the issue:

          https://github.com/apache/flink/pull/3434

          @vasia, I created `PrintableResult` with a `toPrintableString` method and is now used for both algorithms and analytics.

          Show
          githubbot ASF GitHub Bot added a comment - Github user greghogan commented on the issue: https://github.com/apache/flink/pull/3434 @vasia, I created `PrintableResult` with a `toPrintableString` method and is now used for both algorithms and analytics.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vasia commented on the issue:

          https://github.com/apache/flink/pull/3434

          Thanks @greghogan. Looks good!

          Show
          githubbot ASF GitHub Bot added a comment - Github user vasia commented on the issue: https://github.com/apache/flink/pull/3434 Thanks @greghogan. Looks good!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/3434

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3434
          Hide
          greghogan Greg Hogan added a comment -

          Implemented in 33cd97953a7943437acb925ad3296a1eb9858c73

          Show
          greghogan Greg Hogan added a comment - Implemented in 33cd97953a7943437acb925ad3296a1eb9858c73

            People

            • Assignee:
              greghogan Greg Hogan
              Reporter:
              greghogan Greg Hogan
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development