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

add graph_datacell and sparse_graph_datacell #96

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

LHT129
Copy link
Collaborator

@LHT129 LHT129 commented Oct 28, 2024

issue: #40

  • graph_datacell for bottom graph
  • sparse_graph_datacell for high level graph

@LHT129 LHT129 added the kind/feature New feature or request label Oct 28, 2024
@LHT129 LHT129 self-assigned this Oct 28, 2024
@LHT129 LHT129 requested a review from jiaweizone as a code owner October 28, 2024 02:34
@LHT129 LHT129 force-pushed the graph_datacell branch 3 times, most recently from bc3dfbe to 68ac1da Compare October 31, 2024 05:56
src/data_cell/graph_datacell.h Outdated Show resolved Hide resolved
src/data_cell/graph_datacell.h Show resolved Hide resolved
src/data_cell/graph_datacell.h Outdated Show resolved Hide resolved
src/data_cell/graph_datacell.h Outdated Show resolved Hide resolved
src/data_cell/graph_datacell_test.cpp Outdated Show resolved Hide resolved
src/data_cell/sparse_graph_datacell.h Outdated Show resolved Hide resolved
src/data_cell/sparse_graph_datacell.h Outdated Show resolved Hide resolved
src/data_cell/sparse_graph_datacell.cpp Outdated Show resolved Hide resolved
src/data_cell/graph_interface_test.h Outdated Show resolved Hide resolved
src/data_cell/sparse_graph_datacell_test.cpp Outdated Show resolved Hide resolved
@LHT129 LHT129 force-pushed the graph_datacell branch 12 times, most recently from b64b58a to 694bb38 Compare October 31, 2024 09:21
src/data_cell/graph_datacell.h Show resolved Hide resolved
src/data_cell/sparse_graph_datacell.h Show resolved Hide resolved
src/data_cell/graph_datacell.h Outdated Show resolved Hide resolved
@LHT129 LHT129 force-pushed the graph_datacell branch 2 times, most recently from 0b8c3da to c8f2436 Compare November 1, 2024 06:25
Copy link
Collaborator

@wxyucs wxyucs left a comment

Choose a reason for hiding this comment

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

lgtm

src/data_cell/flatten_datacell.h Outdated Show resolved Hide resolved
src/data_cell/graph_datacell.h Show resolved Hide resolved
};

template <typename IOTmpl>
GraphDataCell<IOTmpl, false>::GraphDataCell(const JsonType& graph_json_params,
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor: graph_json_params and simply to graph_params

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

src/data_cell/graph_datacell.h Show resolved Hide resolved
namespace vsag {

GraphInterfacePtr
GraphInterface::MakeInstance(const JsonType& json_obj,
Copy link
Collaborator

Choose a reason for hiding this comment

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

json_obj -> build_conf / build_params ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

return std::make_shared<GraphDataCell<MemoryBlockIO, false>>(
graph_param, io_param, common_param);
}
if (io_string == IO_TYPE_VALUE_MEMORY_IO) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor: else if

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

src/data_cell/graph_interface.h Show resolved Hide resolved

namespace vsag {

SparseGraphDataCell::SparseGraphDataCell(const JsonType& graph_json_params,
Copy link
Collaborator

Choose a reason for hiding this comment

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

simply to graph_params ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done


SparseGraphDataCell::SparseGraphDataCell(const JsonType& graph_json_params,
const IndexCommonParam& common_param)
: allocator_(common_param.allocator_), neighbors_(allocator_) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor: Here depends the fields initialize sequence
can change to: neighbors_(common_param.allocator_)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

src/data_cell/sparse_graph_datacell.cpp Show resolved Hide resolved
- graph_datacell for bottom graph
- sparse_graph_datacell for high level graph

Signed-off-by: LHT129 <tianlan.lht@antgroup.com>
Copy link
Collaborator

@jiaweizone jiaweizone left a comment

Choose a reason for hiding this comment

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

LGTM

@LHT129 LHT129 merged commit 385a5d9 into antgroup:main Nov 4, 2024
2 checks passed
@jiaweizone jiaweizone mentioned this pull request Nov 4, 2024
@LHT129 LHT129 deleted the graph_datacell branch January 17, 2025 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants