Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes in computing function endpoints for replication cases #9632

Merged
merged 2 commits into from
Apr 10, 2019
Merged

Fixes in computing function endpoints for replication cases #9632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f36d588
fixes in computing function endpoints for replication
aparajit-pratap Apr 8, 2019
d9b3819
add mixed option fix to case 6
aparajit-pratap Apr 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions src/Engine/ProtoCore/Lang/CallSite.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@ private FunctionEndPoint GetLooseCompliantFEP(
/// </summary>
/// <returns>Returns true or false based on the condition described above.
/// </returns>
private Boolean IsSimilarOptionButOfHigherRank(List<ReplicationInstruction> oldOption, List<ReplicationInstruction> newOption)
private static bool IsSimilarOptionButOfHigherRank(List<ReplicationInstruction> oldOption, List<ReplicationInstruction> newOption)
{
if (oldOption.Count > 0 && newOption.Count > 0 && oldOption.Count < newOption.Count)
{
Expand Down Expand Up @@ -879,7 +879,7 @@ private void ComputeFeps(
{
if (replicationInstructions == null || IsSimilarOptionButOfHigherRank(replicationInstructions, replicationOption))
{
//Otherwise we have a cluster of FEPs that can be used to dispatch the array
// We have a cluster of FEPs that can be used to dispatch the array
resolvedFeps = new List<FunctionEndPoint>(lookups);
replicationInstructions = replicationOption;
matchFound = true;
Expand Down Expand Up @@ -907,14 +907,18 @@ private void ComputeFeps(
{
if (arguments.Any(arg => arg.IsArray))
{
foreach (List<ReplicationInstruction> replicationOption in replicationTrials)
foreach (var replicationOption in replicationTrials)
{
FunctionEndPoint compliantTarget = GetCompliantFEP(context, arguments, funcGroup, replicationOption, stackFrame, runtimeCore);
if (compliantTarget != null)
{
resolvedFeps = new List<FunctionEndPoint>() { compliantTarget };
replicationInstructions = replicationOption;
matchFound = true;
if (replicationInstructions == null ||
IsSimilarOptionButOfHigherRank(replicationInstructions, replicationOption))
{
resolvedFeps = new List<FunctionEndPoint>() {compliantTarget};
replicationInstructions = replicationOption;
matchFound = true;
}
}
}
if (matchFound)
Expand Down Expand Up @@ -949,9 +953,13 @@ private void ComputeFeps(
FunctionEndPoint compliantTarget = GetLooseCompliantFEP(context, arguments, funcGroup, replicationOption, stackFrame, runtimeCore);
if (compliantTarget != null)
{
resolvedFeps = new List<FunctionEndPoint>() { compliantTarget };
replicationInstructions = replicationOption;
matchFound = true;
if (replicationInstructions == null ||
IsSimilarOptionButOfHigherRank(replicationInstructions, replicationOption))
{
resolvedFeps = new List<FunctionEndPoint>() {compliantTarget};
replicationInstructions = replicationOption;
matchFound = true;
}
}
}
if (matchFound)
Expand Down
39 changes: 21 additions & 18 deletions src/Engine/ProtoCore/Lang/Replication/ReplicationInstruction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,38 +28,41 @@ public struct ReplicationInstruction

public override string ToString()
{
if (!Zipped)
return "Cartesian, Index: " + CartesianIndex;
else
{
string indecies = "";
if (!Zipped) return "Cartesian, Index: " + CartesianIndex;

for (int i = 0; i < ZipIndecies.Count - 1; i++)
indecies += ZipIndecies[i] + ", ";
string indices = "";

indecies += ZipIndecies[ZipIndecies.Count - 1];
for (int i = 0; i < ZipIndecies.Count - 1; i++)
{
indices += ZipIndecies[i] + ", ";
}

indecies += " - " + ZipAlgorithm;
indices += ZipIndecies[ZipIndecies.Count - 1];

return "Zipped, indecies: " + indecies;
}
indices += " - " + ZipAlgorithm;

return "Zipped, indecies: " + indices;

}

public Boolean Equals(ReplicationInstruction oldOption) {
public Boolean Equals(ReplicationInstruction other) {

if (this.Zipped == oldOption.Zipped && this.ZipAlgorithm == oldOption.ZipAlgorithm)
if (this.Zipped == other.Zipped && this.ZipAlgorithm == other.ZipAlgorithm)
{
if (this.ZipIndecies == null && oldOption.ZipIndecies == null)
return true;
if (this.ZipIndecies == null && other.ZipIndecies == null)
{
if (this.CartesianIndex == other.CartesianIndex) return true;

return false;
}

if (this.ZipIndecies != null && oldOption.ZipIndecies != null)
if (this.ZipIndecies != null && other.ZipIndecies != null)
{
// Fastest way to compare all elements of 2 lists.
// Excluding one list from another list and checking if the leftover lists is empty or not.
// https://stackoverflow.com/questions/12795882/quickest-way-to-compare-two-list
var currentExcludesOldList = this.ZipIndecies.Except(oldOption.ZipIndecies).ToList();
var oldExcludescurrentList = oldOption.ZipIndecies.Except(this.ZipIndecies).ToList();
var currentExcludesOldList = this.ZipIndecies.Except(other.ZipIndecies).ToList();
var oldExcludescurrentList = other.ZipIndecies.Except(this.ZipIndecies).ToList();

return !currentExcludesOldList.Any() && !oldExcludescurrentList.Any();
}
Expand Down
63 changes: 60 additions & 3 deletions test/Engine/ProtoTest/TD/Associative/Replication.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4927,7 +4927,7 @@ public void TestReplicationInInlineConditional()
thisTest.Verify("r", new object[] { 2, 5, 7 });
}

// This tests the case 2 block in the computeFeps method(CallSite.cs line:943)
// This tests the case 2 block in the computeFeps method (CallSite.cs)
[Test]
public void TestReplicationWithEmptyListInNestedLists()
{
Expand All @@ -4944,7 +4944,7 @@ public void TestReplicationWithEmptyListInNestedLists()
thisTest.Verify("px2", new object[] { new object[] { 0 }, new object[] { } });
}

// This tests the case 4 block in the computeFeps method(CallSite.cs line:943)
// This tests the case 4 block in the computeFeps method (CallSite.cs)
[Test]
public void TestReplicationWithNullElementInNestedLists()
{
Expand All @@ -4962,7 +4962,7 @@ public void TestReplicationWithNullElementInNestedLists()
thisTest.Verify("px2", new object[] { new object[] { 0 }, null });
}

// This tests the case:6 block in the computeFeps method(CallSite.cs line:943)
// This tests the case:6 block in the computeFeps method (CallSite.cs)
// input example for case 6: l4 and l5 lists.
[Test]
public void TestReplicationWithArraysOfDifferentRanks()
Expand Down Expand Up @@ -4990,6 +4990,63 @@ public void TestReplicationWithArraysOfDifferentRanks()
thisTest.Verify("px4", new object[] { null, new object[] { 0 }, new object[] { new object[] { 0 } } });
thisTest.Verify("px5", new object[] { null, new object[] { new object[] { 0 } }, new object[] { 0 } });
}

// This tests the case 4 block (with type conversion) in the computeFeps method (CallSite.cs)
[Test]
public void TestReplicationWithMixedOptionTypeConversion()
{
string code = @"
def foo ( a : double[], b :double[] )
{
return = Count(a) + Count(b);
}
a = [ 1, 2 ];
b = [[3, 4], [5,9]];
c = b[[0.1,1.1]][0..1];
test = foo (c, a[0..1]);";
Copy link
Member

@mjkkirschner mjkkirschner Apr 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't a[0..1] just equal to a?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes they are same.


var mirror = thisTest.RunScriptSource(code);
thisTest.Verify("c", new object[] {new[] {3, 4}, new[] {5, 9}});
thisTest.Verify("test", new[] {4, 4});
}

// This tests the case 2 block in the computeFeps method (CallSite.cs)
[Test]
public void TestReplicationWithMixedOptionExactMatch()
{
string code = @"
def foo ( a : double[], b :double[] )
{
return = Count(a) + Count(b);
}
a = [ 1, 2 ];
b = [[3, 4], [5,9]];
c = b[[0,1]][0..1];
test = foo (c,a[0..1]);";

var mirror = thisTest.RunScriptSource(code);
thisTest.Verify("c", new object[] { new[] { 3, 4 }, new[] { 5, 9 } });
thisTest.Verify("test", new[] { 4, 4 });
}

// This tests the case 6 block in the computeFeps method (CallSite.cs)
[Test]
public void TestReplicationWithNonConvertibles()
{
string code = @"
def foo ( a : double[], b :double[] )
{
return = Count(a) + Count(b);
}
a = [ 1, 2 ];
b = [[3, 4], [5,9]];
c = b[[true,0]][0..1]; // [[b[true][0], b[true, 1]], [b[0,0], b[0,1]]] => [null, [3, 4]]
test = foo (c, a[0..1]); ";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor change, extra spaces here at the end of the string.

var mirror = thisTest.RunScriptSource(code);
thisTest.Verify("c", new object[] { null, new[] { 3, 4 } });
thisTest.Verify("test", new[] { 3, 4 });

}
}
}