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 hgraph param #111

Merged
merged 1 commit into from
Nov 6, 2024
Merged

add hgraph param #111

merged 1 commit into from
Nov 6, 2024

Conversation

LHT129
Copy link
Collaborator

@LHT129 LHT129 commented Nov 4, 2024

issue: #40

  • introduce inner param for inner indexes
  • map the external param to inner
  • add format_map for long string to help reading

@LHT129 LHT129 requested a review from inabao November 4, 2024 09:23
@LHT129 LHT129 self-assigned this Nov 4, 2024
@LHT129 LHT129 requested a review from ShawnShawnYou November 4, 2024 09:23
@LHT129 LHT129 added the kind/feature New feature or request label Nov 4, 2024
@LHT129 LHT129 force-pushed the hgraph_params branch 3 times, most recently from bde288f to 735c3d7 Compare November 4, 2024 12:28
src/index/hgraph_zparameters.cpp Outdated Show resolved Hide resolved
}
}

const std::unordered_map<std::string, std::vector<std::string>> HgraphParameters::EXTERNAL_MAPPING =
Copy link
Collaborator

Choose a reason for hiding this comment

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

move to L25 ?
let the variables definition precede functions definition

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/index/hgraph_zparameters.h Outdated Show resolved Hide resolved
src/index/hgraph_zparameters.h Outdated Show resolved Hide resolved
}

void
sync_string_by_json() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

refresh_string ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ditto

src/utils.cpp Show resolved Hide resolved
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

- introduce inner param for inner indexes
- map the exteral param to inner
- add format_map for long string to help reading

Signed-off-by: LHT129 <[email protected]>
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

@LHT129 LHT129 merged commit 89ae0a6 into antgroup:main Nov 6, 2024
2 checks passed
@LHT129 LHT129 deleted the hgraph_params branch January 17, 2025 06:39
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/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants