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

introduce new eval tool: eval_performance #234

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Conversation

LHT129
Copy link
Collaborator

@LHT129 LHT129 commented Dec 19, 2024

  • the same as test_performance but support more
  • add Monitor, EvalCase to extends the eval
  • use argparse for command line

@LHT129 LHT129 self-assigned this Dec 19, 2024
@LHT129 LHT129 added the kind/improvement Code improvements (variable/function renaming, refactoring, etc. ) label Dec 19, 2024
@LHT129 LHT129 force-pushed the eval branch 2 times, most recently from 57e1fa9 to f5bf68f Compare December 23, 2024 06:43
@LHT129 LHT129 force-pushed the eval branch 4 times, most recently from 6be5a00 to a3fc5cd Compare December 27, 2024 06:21
tools/eval/build_eval_case.cpp Outdated Show resolved Hide resolved
tools/eval/build_eval_case.h Outdated Show resolved Hide resolved
tools/eval/search_eval_case.h Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jan 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

@@            Coverage Diff             @@
##             main     #234      +/-   ##
==========================================
- Coverage   90.97%   90.15%   -0.82%     
==========================================
  Files         114      116       +2     
  Lines        6947     7022      +75     
==========================================
+ Hits         6320     6331      +11     
- Misses        627      691      +64     
Flag Coverage Δ
cpp 90.15% <ø> (-0.82%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
common 86.82% <16.00%> (-5.83%) ⬇️
datacell 90.18% <66.66%> (-0.16%) ⬇️
index 90.90% <ø> (+0.02%) ⬆️
simd 100.00% <ø> (ø)

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 e587206...c38a4ec. Read the comment docs.

@LHT129 LHT129 force-pushed the eval branch 3 times, most recently from 6394d45 to 21461fd Compare January 9, 2025 01:16
@LHT129 LHT129 force-pushed the eval branch 3 times, most recently from 1e086b5 to 1a02778 Compare January 13, 2025 09:15
@LHT129 LHT129 force-pushed the eval branch 2 times, most recently from 9e50409 to ec7edd0 Compare January 16, 2025 03:27
Copy link
Collaborator

@inabao inabao left a comment

Choose a reason for hiding this comment

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

LGTM


argparse::ArgumentParser& parser_;
};
} // namespace eval
Copy link
Collaborator

Choose a reason for hiding this comment

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

add new line

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

} else if (type == "search") {
return std::make_shared<SearchEvalCase>(dataset_path, index_path, index.value(), config);
} else {
return nullptr;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, we can directly throw an error and log it, as it seems there is no nullptr check being performed outside.

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

@LHT129 LHT129 requested a review from inabao January 16, 2025 03:46
@LHT129 LHT129 force-pushed the eval branch 4 times, most recently from d1d2610 to dbe0fe3 Compare January 16, 2025 10:24
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

Copy link
Collaborator

@inabao inabao left a comment

Choose a reason for hiding this comment

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

LGTM

- the same as test_performance but support more
- add Monitor, EvalCase to extends the eval
- use argparse for command line

Signed-off-by: LHT129 <[email protected]>
@LHT129 LHT129 merged commit d9121cd into antgroup:main Jan 17, 2025
13 of 16 checks passed
@LHT129 LHT129 deleted the eval branch January 17, 2025 06:28
Carrot-77 pushed a commit to Carrot-77/vsag that referenced this pull request Jan 20, 2025
- the same as test_performance but support more
- add Monitor, EvalCase to extends the eval
- use argparse for command line

Signed-off-by: LHT129 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/improvement Code improvements (variable/function renaming, refactoring, etc. ) module/tools size/XXL version/0.13
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants