Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-6382

Support additional types for generated graphs in Gelly examples

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Implemented
    • Affects Version/s: 1.3.0
    • Fix Version/s: 1.3.0
    • Component/s: Gelly
    • Labels:
      None

      Description

      The Gelly examples current support IntValue, LongValue, and StringValue for RMatGraph. Allow transformations and tests for all generated graphs for ByteValue, Byte, ShortValue, Short, CharValue, Character, Integer, Long, and String.

      This is additionally of interest for benchmarking and testing modifications to Flink's internal sort.

        Issue Links

          Activity

          Hide
          heytitle Pattarawat Chormai added a comment -

          Hi Greg Hogan,

          Since I've been already involving on improving sort performance, I can help you tackle this issue.

          Best,
          Pat

          Show
          heytitle Pattarawat Chormai added a comment - Hi Greg Hogan , Since I've been already involving on improving sort performance, I can help you tackle this issue. Best, Pat
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user greghogan opened a pull request:

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

          FLINK-6382 [gelly] Support all numeric types for generated graphs in Gelly examples

          The Gelly examples current support IntValue, LongValue, and StringValue for RMatGraph. Allow transformations and tests for all generated graphs for ByteValue, Byte, ShortValue, Short, CharValue, Character, Integer, Long, and String.

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

          $ git pull https://github.com/greghogan/flink 6382_support_all_numeric_types_for_generated_graphs_in_gelly_examples

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

          https://github.com/apache/flink/pull/3779.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 #3779


          commit c1ef02c4df46b127788cee63d080b588b731e696
          Author: Greg Hogan <code@greghogan.com>
          Date: 2017-04-25T15:36:08Z

          FLINK-6382 [gelly] Support all numeric types for generated graphs in Gelly examples

          The Gelly examples current support IntValue, LongValue, and StringValue
          for RMatGraph. Allow transformations and tests for all generated graphs
          for ByteValue, Byte, ShortValue, Short, CharValue, Character, Integer,
          Long, and String.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/3779 FLINK-6382 [gelly] Support all numeric types for generated graphs in Gelly examples The Gelly examples current support IntValue, LongValue, and StringValue for RMatGraph. Allow transformations and tests for all generated graphs for ByteValue, Byte, ShortValue, Short, CharValue, Character, Integer, Long, and String. You can merge this pull request into a Git repository by running: $ git pull https://github.com/greghogan/flink 6382_support_all_numeric_types_for_generated_graphs_in_gelly_examples Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3779.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 #3779 commit c1ef02c4df46b127788cee63d080b588b731e696 Author: Greg Hogan <code@greghogan.com> Date: 2017-04-25T15:36:08Z FLINK-6382 [gelly] Support all numeric types for generated graphs in Gelly examples The Gelly examples current support IntValue, LongValue, and StringValue for RMatGraph. Allow transformations and tests for all generated graphs for ByteValue, Byte, ShortValue, Short, CharValue, Character, Integer, Long, and String.
          Hide
          greghogan Greg Hogan added a comment -

          Hi Pattarawat Chormai, it would be great if you could review the PR. I will use this PR to test FLINK-3722 in a local branch so NormalizedKeySorter changes can be merged and FLINK-5734 rebased.

          Show
          greghogan Greg Hogan added a comment - Hi Pattarawat Chormai , it would be great if you could review the PR. I will use this PR to test FLINK-3722 in a local branch so NormalizedKeySorter changes can be merged and FLINK-5734 rebased.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user heytitle commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3779#discussion_r114009718

          — Diff: flink-libraries/flink-gelly-examples/src/test/java/org/apache/flink/graph/drivers/DriverBaseITCase.java —
          @@ -41,17 +43,41 @@
          @Rule
          public ExpectedException expectedException = ExpectedException.none();

          • protected DriverBaseITCase(TestExecutionMode mode) {
            + protected final String idType;
            +
            + protected DriverBaseITCase(TestExecutionMode mode, String idType) { super(mode); + + this.idType = idType; }
          • // extend MultipleProgramsTestBase default to include object reuse mode
          • @Parameterized.Parameters(name = "Execution mode = {0}")
            + @Parameterized.Parameters(name = "Execution mode = {0}

            , ID type =

            {1}

            ")
            public static Collection<Object[]> executionModes() {

          • return Arrays.asList(
          • new Object[] { TestExecutionMode.CLUSTER }

            ,

          • new Object[] { TestExecutionMode.CLUSTER_OBJECT_REUSE }

            ,

          • new Object[] { TestExecutionMode.COLLECTION }

            );
            + List<Object[]> executionModes = new ArrayList<>();
            +
            + for (TestExecutionMode executionMode : TestExecutionMode.values()) {
            + for (String idType : new String[]

            {"byte", "nativeByte", "short", "nativeShort", "char", "nativeChar", + "integer", "nativeInteger", "long", "nativeLong", "string", "nativeString"}

            ) {

              • End diff –

          What do you think if we declare the list of `idType` before the outer loop?
          IMHO, this will make formation better.

          Show
          githubbot ASF GitHub Bot added a comment - Github user heytitle commented on a diff in the pull request: https://github.com/apache/flink/pull/3779#discussion_r114009718 — Diff: flink-libraries/flink-gelly-examples/src/test/java/org/apache/flink/graph/drivers/DriverBaseITCase.java — @@ -41,17 +43,41 @@ @Rule public ExpectedException expectedException = ExpectedException.none(); protected DriverBaseITCase(TestExecutionMode mode) { + protected final String idType; + + protected DriverBaseITCase(TestExecutionMode mode, String idType) { super(mode); + + this.idType = idType; } // extend MultipleProgramsTestBase default to include object reuse mode @Parameterized.Parameters(name = "Execution mode = {0}") + @Parameterized.Parameters(name = "Execution mode = {0} , ID type = {1} ") public static Collection<Object[]> executionModes() { return Arrays.asList( new Object[] { TestExecutionMode.CLUSTER } , new Object[] { TestExecutionMode.CLUSTER_OBJECT_REUSE } , new Object[] { TestExecutionMode.COLLECTION } ); + List<Object[]> executionModes = new ArrayList<>(); + + for (TestExecutionMode executionMode : TestExecutionMode.values()) { + for (String idType : new String[] {"byte", "nativeByte", "short", "nativeShort", "char", "nativeChar", + "integer", "nativeInteger", "long", "nativeLong", "string", "nativeString"} ) { End diff – What do you think if we declare the list of `idType` before the outer loop? IMHO, this will make formation better.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user heytitle commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3779#discussion_r114007404

          — Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/drivers/input/GridGraph.java —
          @@ -82,11 +81,27 @@ public void configure(ParameterTool parameterTool) throws ProgramParametrization

          @Override
          public String getIdentity()

          { - return getName() + " (" + dimensions + ")"; + return getTypeName() + " " + getName() + " (" + dimensions + ")"; }

          @Override

          • public Graph<LongValue, NullValue, NullValue> create(ExecutionEnvironment env) {
            + protected long vertexCount() {
            + // in Java 8 use Math.multiplyExact(long, long)
              • End diff –

          What is the purpose of this comment?

          Show
          githubbot ASF GitHub Bot added a comment - Github user heytitle commented on a diff in the pull request: https://github.com/apache/flink/pull/3779#discussion_r114007404 — Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/drivers/input/GridGraph.java — @@ -82,11 +81,27 @@ public void configure(ParameterTool parameterTool) throws ProgramParametrization @Override public String getIdentity() { - return getName() + " (" + dimensions + ")"; + return getTypeName() + " " + getName() + " (" + dimensions + ")"; } @Override public Graph<LongValue, NullValue, NullValue> create(ExecutionEnvironment env) { + protected long vertexCount() { + // in Java 8 use Math.multiplyExact(long, long) End diff – What is the purpose of this comment?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user heytitle commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3779#discussion_r114010412

          — Diff: flink-libraries/flink-gelly/src/main/java/org/apache/flink/graph/library/link_analysis/HITS.java —
          @@ -60,8 +60,6 @@

          Nice catch!

          Show
          githubbot ASF GitHub Bot added a comment - Github user heytitle commented on a diff in the pull request: https://github.com/apache/flink/pull/3779#discussion_r114010412 — Diff: flink-libraries/flink-gelly/src/main/java/org/apache/flink/graph/library/link_analysis/HITS.java — @@ -60,8 +60,6 @@ <p> http://www.cs.cornell.edu/home/kleinber/auth.pdf * * http://www.cs.cornell.edu/home/kleinber/auth.pdf * End diff – Nice catch!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user greghogan commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3779#discussion_r114030564

          — Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/drivers/input/GridGraph.java —
          @@ -82,11 +81,27 @@ public void configure(ParameterTool parameterTool) throws ProgramParametrization

          @Override
          public String getIdentity()

          { - return getName() + " (" + dimensions + ")"; + return getTypeName() + " " + getName() + " (" + dimensions + ")"; }

          @Override

          • public Graph<LongValue, NullValue, NullValue> create(ExecutionEnvironment env) {
            + protected long vertexCount() {
            + // in Java 8 use Math.multiplyExact(long, long)
              • End diff –

          Rather than the current implementation using `BigInteger`, Java 8's `Math.multiplyExact` detects and throws an exception on overflow. I was noting for the future when the Flink codebase targets Java 8.

          Show
          githubbot ASF GitHub Bot added a comment - Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/3779#discussion_r114030564 — Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/drivers/input/GridGraph.java — @@ -82,11 +81,27 @@ public void configure(ParameterTool parameterTool) throws ProgramParametrization @Override public String getIdentity() { - return getName() + " (" + dimensions + ")"; + return getTypeName() + " " + getName() + " (" + dimensions + ")"; } @Override public Graph<LongValue, NullValue, NullValue> create(ExecutionEnvironment env) { + protected long vertexCount() { + // in Java 8 use Math.multiplyExact(long, long) End diff – Rather than the current implementation using `BigInteger`, Java 8's `Math.multiplyExact` detects and throws an exception on overflow. I was noting for the future when the Flink codebase targets Java 8.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user greghogan commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3779#discussion_r114030670

          — Diff: flink-libraries/flink-gelly-examples/src/test/java/org/apache/flink/graph/drivers/DriverBaseITCase.java —
          @@ -41,17 +43,41 @@
          @Rule
          public ExpectedException expectedException = ExpectedException.none();

          • protected DriverBaseITCase(TestExecutionMode mode) {
            + protected final String idType;
            +
            + protected DriverBaseITCase(TestExecutionMode mode, String idType) { super(mode); + + this.idType = idType; }
          • // extend MultipleProgramsTestBase default to include object reuse mode
          • @Parameterized.Parameters(name = "Execution mode = {0}")
            + @Parameterized.Parameters(name = "Execution mode = {0}

            , ID type =

            {1}

            ")
            public static Collection<Object[]> executionModes() {

          • return Arrays.asList(
          • new Object[] { TestExecutionMode.CLUSTER }

            ,

          • new Object[] { TestExecutionMode.CLUSTER_OBJECT_REUSE }

            ,

          • new Object[] { TestExecutionMode.COLLECTION }

            );
            + List<Object[]> executionModes = new ArrayList<>();
            +
            + for (TestExecutionMode executionMode : TestExecutionMode.values()) {
            + for (String idType : new String[]

            {"byte", "nativeByte", "short", "nativeShort", "char", "nativeChar", + "integer", "nativeInteger", "long", "nativeLong", "string", "nativeString"}

            ) {

              • End diff –

          I've also changed modified the parameters string to print the ID type before the ExecutionMode.

          Show
          githubbot ASF GitHub Bot added a comment - Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/3779#discussion_r114030670 — Diff: flink-libraries/flink-gelly-examples/src/test/java/org/apache/flink/graph/drivers/DriverBaseITCase.java — @@ -41,17 +43,41 @@ @Rule public ExpectedException expectedException = ExpectedException.none(); protected DriverBaseITCase(TestExecutionMode mode) { + protected final String idType; + + protected DriverBaseITCase(TestExecutionMode mode, String idType) { super(mode); + + this.idType = idType; } // extend MultipleProgramsTestBase default to include object reuse mode @Parameterized.Parameters(name = "Execution mode = {0}") + @Parameterized.Parameters(name = "Execution mode = {0} , ID type = {1} ") public static Collection<Object[]> executionModes() { return Arrays.asList( new Object[] { TestExecutionMode.CLUSTER } , new Object[] { TestExecutionMode.CLUSTER_OBJECT_REUSE } , new Object[] { TestExecutionMode.COLLECTION } ); + List<Object[]> executionModes = new ArrayList<>(); + + for (TestExecutionMode executionMode : TestExecutionMode.values()) { + for (String idType : new String[] {"byte", "nativeByte", "short", "nativeShort", "char", "nativeChar", + "integer", "nativeInteger", "long", "nativeLong", "string", "nativeString"} ) { End diff – I've also changed modified the parameters string to print the ID type before the ExecutionMode.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

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

          Implemented in 33695781f9ce2599d18f45de6a465eaefe7d71f4

          Show
          greghogan Greg Hogan added a comment - Implemented in 33695781f9ce2599d18f45de6a465eaefe7d71f4

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development