organize an example pagerank app employing the gar library (#44) #46

andydiwenzhu · 2022-12-16T05:58:07Z

Proposed changes

Organize the original PageRank test code as an example application. It demonstrates how to edit CMake to integrate GAR C++ library and how to extend an input graph with its PageRank values using GAR methods.

Types of changes

What types of changes does your code introduce to GraphAr?
Put an x in the boxes that apply

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation Update (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

I have read the CONTRIBUTING doc
I have signed the CLA
Lint and unit tests pass locally with my changes
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

acezen

Nice work! The pull request looks good. It has basically met the issue requirement but some modification still need.

acezen · 2022-12-16T09:08:44Z

examples/pagerank/pagerank.cc

+
+int main(int argc, char* argv[]) {
+  // read file and construct graph info
+  std::string path = "/tmp/ldbc_sample/parquet/ldbc_sample.graph.yml";


This is a little hard-code. Use the relative path to test/gar-test or add a soft link to it maybe better

My purpose is to make this example a new project over the GAR library.

acezen · 2022-12-16T09:22:10Z

examples/pagerank/pagerank.cc

+
+  // extend the original vertex info and write results to gar using writer
+  // construct property group
+  GraphArchive::Property pagerank = {


As a pagerank example, I don't know if we just write the result to file not extend as a property would be better. That would have two benefits:

simpler and clear for a example

no need to include arrow header any more.
@lixueclaire what do you think?

I agree. To remove arrow here, you can use VertexBuilder instead of VertexPropertyWriter to write results. (please refer to "method 1 for writing results" in https://github.com/alibaba/GraphAr/blob/main/test/test_example/test_bgl_example.cc)

acezen · 2022-12-16T09:29:10Z

examples/pagerank/CMakeLists.txt

@@ -0,0 +1,23 @@
+cmake_minimum_required(VERSION 2.8)


I'm not sure if we need a CMakeList here. Seems put these to CMakeLists in root path and make example as a new target would be better.
A new CMakeList.txt here would bring some cons:

it need user/develop to install GraphAr first, not just make

it's building need to build arrow again.

Yes. I think what you suggested is the in-project examples. What I want to show here is a new application employing the GAR library, in which case, users should first install GAR, and then build their own application projects.

acezen · 2022-12-16T09:29:55Z

examples/pagerank/cmake/apache-arrow.cmake

@@ -0,0 +1,92 @@
+# Licensed to the Apache Software Foundation (ASF) under one


Ditto. We can reuse the apache-arrow.cmake of GraphAr.

I prefer remove arrow from our example

acezen · 2022-12-20T01:48:56Z

examples/bfs_father_example.cc

@@ -6,7 +6,7 @@ You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

-Unless required by applicable law or agreed to in writing, software
+Unless assertd by applicable law or agreed to in writing, software


Sorry, Why we need to change required to assertd here and the assertd has a typo ?

this is a replace mistake, I will revert it back 😁

acezen

Except the required to asserted question, this PR is LGTM

lixueclaire

I have no additional comments except the filenames.

lixueclaire · 2022-12-20T02:39:47Z

docs/applications/out-of-core.rst

@@ -107,16 +107,16 @@ Meanwhile, BFS could be implemented in a **push**-style which only traverses the
 In some cases, it is required to record the path of BFS, that is, to maintain each vertex's predecessor (also called *father*) in the traversing tree rather than only recording the distance. The implementation of BFS with recording fathers can be found at `test_bfs_father_example.cc`_.


-.. _test_pagerank_example.cc: https://github.com/alibaba/GraphAr/blob/main/test/test_example/test_pagerank_example.cc
+.. _test_pagerank_example.cc: https://github.com/alibaba/GraphAr/blob/main/examples/pagerank_example.cc


please modify these file names, for example, "test_pagerank_example.cc" -> "pagerank_example.cc"

lixueclaire · 2022-12-20T02:40:29Z

docs/user-guide/getting-started.rst


-.. _test_pagerank_example.cc: https://github.com/alibaba/GraphAr/blob/main/test/test_example/test_pagerank_example.cc
+.. _test_pagerank_example.cc: https://github.com/alibaba/GraphAr/blob/main/examples/pagerank_example.cc


"test_pagerank_example.cc" -> "pagerank_example.cc"

organize an example pagerank app employing the gar library (apache#44)

498cf3e

andydiwenzhu requested review from acezen and lixueclaire December 16, 2022 06:04

acezen reviewed Dec 16, 2022

View reviewed changes

andydiwenzhu added 2 commits December 18, 2022 18:15

move test_examples to examples

095e137

add cmake exmaple option

1d09b4d

acezen reviewed Dec 20, 2022

View reviewed changes

acezen approved these changes Dec 20, 2022

View reviewed changes

andydiwenzhu added 2 commits December 20, 2022 10:25

minor fix

c1596c3

fix filename

8350fa2

lixueclaire approved these changes Dec 20, 2022

View reviewed changes

fix filenames in doc

c522cb2

andydiwenzhu merged commit 91251e4 into apache:main Dec 20, 2022

andydiwenzhu deleted the 44-add-examples branch December 20, 2022 03:46

lixueclaire mentioned this pull request Mar 6, 2023

Revise the application example implementation #44

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

organize an example pagerank app employing the gar library (#44) #46

organize an example pagerank app employing the gar library (#44) #46

andydiwenzhu commented Dec 16, 2022

acezen left a comment

acezen Dec 16, 2022

andydiwenzhu Dec 16, 2022

acezen Dec 16, 2022

andydiwenzhu Dec 16, 2022

lixueclaire Dec 16, 2022

acezen Dec 16, 2022

andydiwenzhu Dec 16, 2022

acezen Dec 16, 2022

andydiwenzhu Dec 16, 2022

acezen Dec 20, 2022

andydiwenzhu Dec 20, 2022

acezen left a comment

lixueclaire left a comment

lixueclaire Dec 20, 2022

lixueclaire Dec 20, 2022

		@@ -0,0 +1,92 @@
		# Licensed to the Apache Software Foundation (ASF) under one


		.. _test_pagerank_example.cc: https://github.com/alibaba/GraphAr/blob/main/test/test_example/test_pagerank_example.cc
		.. _test_pagerank_example.cc: https://github.com/alibaba/GraphAr/blob/main/examples/pagerank_example.cc

organize an example pagerank app employing the gar library (#44) #46

organize an example pagerank app employing the gar library (#44) #46

Conversation

andydiwenzhu commented Dec 16, 2022

Proposed changes

Types of changes

Checklist

Further comments

acezen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acezen left a comment

Choose a reason for hiding this comment

lixueclaire left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment