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.

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

feat(interactive): Support Unfold step in GIE #3143

Merged
merged 56 commits into from Sep 6, 2023
Merged

Conversation

Louyk14
Copy link
Collaborator

@Louyk14 Louyk14 commented Aug 24, 2023

What do these changes do?

support Unfold step in GIE

Related issue number

#3129

Fixes

louyunkai and others added 30 commits August 15, 2023 08:15
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Add union+identity test && Identity e2e Test
Committed-by: Yunkai Lou from Dev container
remove std output in test
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
.gitignore Outdated
@@ -69,6 +69,7 @@ interactive_engine/gaia-adaptor/dependency-reduced-pom.xml
interactive_engine/executor/Cargo.lock
interactive_engine/executor/engine/pegasus/benchmark/src/graph/storage/clickhouse/pb_gen/*
interactive_engine/executor/ir/Cargo.lock
interactive_engine/compiler/core
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not necessary.

@@ -0,0 +1,3399 @@
//
//! Copyright 2020 Alibaba Group Holding Limited.
Copy link
Collaborator

Choose a reason for hiding this comment

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

this file should not be included.

@LoadGraphWith(LoadGraphWith.GraphData.MODERN)
@Test
public void g_V_fold_a_unfold_values() {
Traversal<Vertex, Object> traversal = this.get_g_V_fold_a_unfold_values();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we check whether the tag a is still in the record of the unfolded results?

louyunkai and others added 12 commits August 28, 2023 10:00
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
Committed-by: Yunkai Lou from Dev container
longbinlai
longbinlai previously approved these changes Aug 29, 2023
louyunkai and others added 2 commits August 31, 2023 10:37
Committed-by: Yunkai Lou from Dev container
add the optimization about select.as.unfold as a strategy
shirly121
shirly121 previously approved these changes Aug 31, 2023
// System.out.println(step);
// System.out.println(opList);
// System.out.println(step.getLabels());

Copy link
Collaborator

Choose a reason for hiding this comment

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

是否可以将这段代码实现为一个strategy?我理解这里做的事情是将select('a').unfold()的组合转换为unfold(tag='a'),这个看起来更适合实现为strategy,具体可以参考实现https://github.com/alibaba/GraphScope/blob/main/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/intermediate/strategy/TopKStrategy.java

louyunkai and others added 2 commits August 31, 2023 13:08
Committed-by: Yunkai Lou from Dev container
@codecov-commenter
Copy link

codecov-commenter commented Aug 31, 2023

Codecov Report

Merging #3143 (81f73ce) into main (70343f2) will decrease coverage by 0.24%.
Report is 1 commits behind head on main.
The diff coverage is 70.39%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3143      +/-   ##
==========================================
- Coverage   42.29%   42.05%   -0.24%     
==========================================
  Files         101      101              
  Lines       10907    10985      +78     
==========================================
+ Hits         4613     4620       +7     
- Misses       6294     6365      +71     
Files Changed Coverage Δ
python/graphscope/__init__.py 82.85% <ø> (-0.48%) ⬇️
python/graphscope/tests/unittest/test_lazy.py 0.00% <0.00%> (ø)
python/graphscope/tests/unittest/test_session.py 0.00% <0.00%> (ø)
...thon/graphscope/tests/kubernetes/test_with_mars.py 43.75% <33.33%> (ø)
python/graphscope/deploy/hosts/cluster.py 40.29% <35.00%> (+9.81%) ⬆️
python/graphscope/dataset/io_utils.py 15.94% <50.00%> (ø)
...on/graphscope/tests/kubernetes/test_demo_script.py 66.66% <50.00%> (-17.40%) ⬇️
python/graphscope/deploy/kubernetes/cluster.py 78.16% <68.57%> (+1.39%) ⬆️
python/graphscope/client/session.py 66.25% <71.87%> (-0.41%) ⬇️
python/graphscope/config.py 75.09% <74.80%> (-20.98%) ⬇️
... and 5 more

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 665acd4...81f73ce. Read the comment docs.

@longbinlai longbinlai merged commit 9df8588 into alibaba:main Sep 6, 2023
46 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GIE Compiler] Support unfold() Step in Gremlin Queries
5 participants