You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by vasia <gi...@git.apache.org> on 2016/10/07 11:51:36 UTC
[GitHub] flink pull request #2606: Allow custom convergence criterion in delta iterat...
GitHub user vasia opened a pull request:
https://github.com/apache/flink/pull/2606
Allow custom convergence criterion in delta iterations
As discussed in the Jira issue, this PR contains the following changes:
1. use `TaskConfig.setConvergenceCriterion()` to set the custom, user-defined convergence criterion (like in the case of bulk iteration)
2. add a new method `TaskConfig.setDefaultConvergeCriterion()` to handle the default empty workset convergence
\u200b3. check both criteria in `IterationSynchronizationSinkTask.checkForConvergence()\u200b`
4. expose the custom convergence criterion in `DeltaIteration`
It also contains some minor cleanup and corresponding changes in the `CollectionExecutor`.
The iteration docs already state that custom convergence is possible, so no update needed there ;)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vasia/flink flink-3888
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/2606.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 #2606
----
commit 41327034ae57d99fdde74bf24838d19b5aee31f3
Author: vasia <va...@apache.org>
Date: 2016-10-05T11:49:20Z
[FLINK-3888] allow registering a custom convergence criterion in delta iterations
commit e7ffc368f8542d3afe823b86c23e91379a03e21b
Author: vasia <va...@apache.org>
Date: 2016-10-05T12:06:47Z
[FLINK-3888] cleanups in iterations and aggregators code
commit 3b1a5e55686f665b1c1bb90943b0f853e71eae82
Author: vasia <va...@apache.org>
Date: 2016-10-06T20:25:43Z
[FLINK-3888] add delta convergence criterion in the CollectionExecutor
commit 9f41af544eecae0c37ae9470f4ff26f19b5dbdc0
Author: vasia <va...@apache.org>
Date: 2016-10-06T21:00:38Z
[FLINK-3888] add ITCases for delta custom convergence
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2606: [FLINK-3888] Allow custom convergence criterion in delta ...
Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on the issue:
https://github.com/apache/flink/pull/2606
Thank you for the review @greghogan! I have addressed your comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2606: [FLINK-3888] Allow custom convergence criterion in...
Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/2606#discussion_r83632808
--- Diff: flink-optimizer/src/main/java/org/apache/flink/optimizer/plantranslate/JobGraphGenerator.java ---
@@ -1513,14 +1513,21 @@ private void finalizeWorksetIteration(IterationDescriptor descr) {
String convAggName = aggs.getConvergenceCriterionAggregatorName();
ConvergenceCriterion<?> convCriterion = aggs.getConvergenceCriterion();
-
+
if (convCriterion != null || convAggName != null) {
- throw new CompilerException("Error: Cannot use custom convergence criterion with workset iteration. Workset iterations have implicit convergence criterion where workset is empty.");
+ if (convCriterion == null) {
+ throw new CompilerException("Error: Convergence criterion aggregator set, but criterion is null.");
+ }
+ if (convAggName == null) {
+ throw new CompilerException("Error: Aggregator convergence criterion set, but aggregator is null.");
+ }
+
+ syncConfig.setConvergenceCriterion(convAggName, convCriterion);
}
headConfig.addIterationAggregator(WorksetEmptyConvergenceCriterion.AGGREGATOR_NAME, new LongSumAggregator());
syncConfig.addIterationAggregator(WorksetEmptyConvergenceCriterion.AGGREGATOR_NAME, new LongSumAggregator());
- syncConfig.setConvergenceCriterion(WorksetEmptyConvergenceCriterion.AGGREGATOR_NAME, new WorksetEmptyConvergenceCriterion());
+ syncConfig.setDefaultConvergenceCriterion(WorksetEmptyConvergenceCriterion.AGGREGATOR_NAME, new WorksetEmptyConvergenceCriterion());
--- End diff --
Sure that's possible, but each iteration will have its own TaskConfig.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2606: [FLINK-3888] Allow custom convergence criterion in...
Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on a diff in the pull request:
https://github.com/apache/flink/pull/2606#discussion_r83071641
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/iterative/task/IterationSynchronizationSinkTask.java ---
@@ -56,11 +56,15 @@
private SyncEventHandler eventHandler;
private ConvergenceCriterion<Value> convergenceCriterion;
+
+ private ConvergenceCriterion<Value> defaultConvergenceCriterion;
--- End diff --
Perhaps "implicit" instead of "default"? I feel like a default can be overridden, but the use here is for a system configured convergence implicit in how a delta iteration operates.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2606: [FLINK-3888] Allow custom convergence criterion in...
Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on a diff in the pull request:
https://github.com/apache/flink/pull/2606#discussion_r83075316
--- Diff: flink-tests/src/test/java/org/apache/flink/test/iterative/aggregators/AggregatorConvergenceITCase.java ---
@@ -137,42 +138,51 @@ public void testConnectedComponentsWithParametrizableConvergence() {
fail(e.getMessage());
}
}
+
+ @Test
+ public void testDeltaConnectedComponentsWithParametrizableConvergence() {
+ try {
+
+ // name of the aggregator that checks for convergence
+ final String UPDATED_ELEMENTS = "updated.elements.aggr";
+
+ // the iteration stops if less than this number of elements change value
+ final long convergence_threshold = 3;
+
+ final ExecutionEnvironment env = ExecutionEnvironment.getExecutionEnvironment();
+
+ DataSet<Tuple2<Long, Long>> initialSolutionSet = env.fromCollection(verticesInput);
+ DataSet<Tuple2<Long, Long>> edges = env.fromCollection(edgesInput);
+
+ DeltaIteration<Tuple2<Long, Long>, Tuple2<Long, Long>> iteration =
+ initialSolutionSet.iterateDelta(initialSolutionSet, 10, 0);
+
+ // register the convergence criterion
+ iteration.registerAggregationConvergenceCriterion(UPDATED_ELEMENTS,
+ new LongSumAggregator(), new UpdatedElementsConvergenceCriterion(convergence_threshold));
+
+ DataSet<Tuple2<Long, Long>> verticesWithNewComponents = iteration.getWorkset().join(edges).where(0).equalTo(0)
+ .with(new NeighborWithComponentIDJoin())
+ .groupBy(0).min(1);
+
+ DataSet<Tuple2<Long, Long>> updatedComponentId =
+ verticesWithNewComponents.join(iteration.getSolutionSet()).where(0).equalTo(0)
+ .flatMap(new MinimumIdFilter(UPDATED_ELEMENTS));
+
+ List<Tuple2<Long, Long>> result = iteration.closeWith(updatedComponentId, updatedComponentId).collect();
+ Collections.sort(result, new JavaProgramTestBase.TupleComparator<Tuple2<Long, Long>>());
+
+ assertEquals(expectedResult, result);
+ }
+ catch (Exception e) {
--- End diff --
Catching and re-throwing exceptions looks to be unnecessary.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2606: [FLINK-3888] Allow custom convergence criterion in...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/2606
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2606: [FLINK-3888] Allow custom convergence criterion in...
Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on a diff in the pull request:
https://github.com/apache/flink/pull/2606#discussion_r83069609
--- Diff: flink-optimizer/src/main/java/org/apache/flink/optimizer/plantranslate/JobGraphGenerator.java ---
@@ -1513,14 +1513,21 @@ private void finalizeWorksetIteration(IterationDescriptor descr) {
String convAggName = aggs.getConvergenceCriterionAggregatorName();
ConvergenceCriterion<?> convCriterion = aggs.getConvergenceCriterion();
-
+
if (convCriterion != null || convAggName != null) {
- throw new CompilerException("Error: Cannot use custom convergence criterion with workset iteration. Workset iterations have implicit convergence criterion where workset is empty.");
+ if (convCriterion == null) {
+ throw new CompilerException("Error: Convergence criterion aggregator set, but criterion is null.");
+ }
+ if (convAggName == null) {
+ throw new CompilerException("Error: Aggregator convergence criterion set, but aggregator is null.");
+ }
+
+ syncConfig.setConvergenceCriterion(convAggName, convCriterion);
}
headConfig.addIterationAggregator(WorksetEmptyConvergenceCriterion.AGGREGATOR_NAME, new LongSumAggregator());
syncConfig.addIterationAggregator(WorksetEmptyConvergenceCriterion.AGGREGATOR_NAME, new LongSumAggregator());
- syncConfig.setConvergenceCriterion(WorksetEmptyConvergenceCriterion.AGGREGATOR_NAME, new WorksetEmptyConvergenceCriterion());
+ syncConfig.setDefaultConvergenceCriterion(WorksetEmptyConvergenceCriterion.AGGREGATOR_NAME, new WorksetEmptyConvergenceCriterion());
--- End diff --
Is it safe to use a fixed name for the aggregator? Could we not have multiple iterations in a single job?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2606: [FLINK-3888] Allow custom convergence criterion in delta ...
Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on the issue:
https://github.com/apache/flink/pull/2606
+1 to merge
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---