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

[multi-label] add parameter 'label' for createinfos #634

Merged
merged 1 commit into from
Oct 28, 2024
Merged

[multi-label] add parameter 'label' for createinfos #634

merged 1 commit into from
Oct 28, 2024

Conversation

Elssky
Copy link
Contributor

@Elssky Elssky commented Sep 24, 2024

No description provided.

@Elssky Elssky mentioned this pull request Sep 24, 2024
Closed
@Elssky

This comment was marked as resolved.

Sign in to view
@Elssky Elssky changed the title feat(c++): write label chunks feat(c++): add paramater 'label' for createinfos Oct 23, 2024
@Elssky Elssky changed the title feat(c++): add paramater 'label' for createinfos feat(c++): add parameter 'label' for createinfos Oct 23, 2024
Elssky
Elssky commented Oct 23, 2024
View reviewed changes
cpp/examples/bgl_example.cc Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified the createinfo function in exmaples to add the label parameter. If there is no label, use an empty array as a placeholder.

cpp/examples/construct_info_example.cc Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above

cpp/examples/snap_dataset_to_graphar.cc Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above

cpp/CMakeLists.txt Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

add a test file for multi-label

cpp/src/graphar/filesystem.cc
Copy link
Contributor Author

Choose a reason for hiding this comment

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

RLE coding for compress the space occupied

cpp/src/graphar/fwd.h Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

label parameter

cpp/src/graphar/util.cc Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

process arrow::boolean data

cpp/test/test_info.cc Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as the example files

@Elssky Elssky changed the title feat(c++): add parameter 'label' for createinfos feat(c++): [multi-label]add parameter 'label' for createinfos Oct 25, 2024
@Elssky Elssky changed the title feat(c++): [multi-label]add parameter 'label' for createinfos [multi-label] add parameter 'label' for createinfos Oct 25, 2024
@Elssky
Copy link
Contributor Author

Elssky commented Oct 25, 2024

@lixueclaire Please review this Pull Request, which adds label in graph/vertexinfos：）

lixueclaire
lixueclaire reviewed Oct 25, 2024
View reviewed changes
cpp/src/graphar/filesystem.cc Outdated
parquet::WriterProperties::Builder builder;
builder.compression(arrow::Compression::type::ZSTD); // enable compression
for (int i = 0; i < column_num; ++i) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It appears that RLE encoding is currently applied to all columns. Could we adjust this so that RLE is used only on the label columns?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for sure!

cpp/src/graphar/general_params.h Outdated
@@ -27,6 +27,7 @@ struct GeneralParams {
static constexpr const char* kDstIndexCol = "_graphArDstIndex";
static constexpr const char* kOffsetCol = "_graphArOffset";
static constexpr const char* kPrimaryCol = "_graphArPrimary";
static constexpr const char* kLabelCol = ":LABEL";
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we consider renaming the column to start with "_graphAr"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, use ":LABEL" because we want to keep the column names consistent with the csv, but of course we can change the name in the arrow table

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the usage example will be commited in next PR

lixueclaire
lixueclaire approved these changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@lixueclaire lixueclaire left a comment

Choose a reason for hiding this comment

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

LGTM now. Thanks for your contribution!

@Elssky
Copy link
Contributor Author

Elssky commented Oct 28, 2024

LGTM now. Thanks for your contribution!

Thanks! Please help merge this PR, currently I don’t have write access😂

@lixueclaire lixueclaire merged commit 0961b64 into apache:main Oct 28, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lixueclaire lixueclaire lixueclaire approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Elssky @lixueclaire