Skip to content

add some basic structure for quantizer, io and simd operators#25

Merged
LHT129 merged 1 commit into
mainfrom
lht_dev
Sep 23, 2024
Merged

add some basic structure for quantizer, io and simd operators#25
LHT129 merged 1 commit into
mainfrom
lht_dev

Conversation

@LHT129

@LHT129 LHT129 commented Sep 12, 2024

Copy link
Copy Markdown
Collaborator
  • only header for quantizer and io
  • fp32 quantizer as example

Comment thread src/io/memory_io.h Outdated

private:
[[nodiscard]] inline bool
CheckValidOffset(uint64_t size) const {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

unify the naming convention; our private methods generally use the underscore naming style. (check_valid_offset)

Comment thread src/simd/fp32_simd_test.cpp Outdated
#include "fixtures.h"
using namespace vsag;

#define TEST_RECALL(Func) \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Renaming it to TEST_ACCURACY would be more accurate.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread src/simd/generic.cpp
auto val = query[i] - codes[i];
result += val * val;
}
return result;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There is no sqrt in the result. So, it is better to rename it to "FP32ComputeL2".

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

L2Sqr is the Square of L2 norm

private:
uint64_t dim_{0};

uint64_t codeSize_{0};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

unify the code style.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

codeSize -> code_size_

@LHT129
LHT129 force-pushed the lht_dev branch 2 times, most recently from f1ae5ea to ca3eda5 Compare September 12, 2024 09:36
@LHT129
LHT129 requested a review from inabao September 13, 2024 02:46
@LHT129
LHT129 force-pushed the lht_dev branch 2 times, most recently from eb24b83 to 414ce55 Compare September 13, 2024 02:59
auto* outVec = new float[dim];
quant.DecodeOne(codes, outVec);
for (int i = 0; i < dim; ++i) {
REQUIRE(std::abs(vecs[idx * dim + i] - outVec[i]) < error);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should replace abs with fabs?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

For integral arguments, the integral overloads of std::abs are likely better matches.
reference: https://en.cppreference.com/w/cpp/numeric/math/fabs

Comment thread src/simd/avx.cpp
sum = _mm256_add_ps(sum, _mm256_mul_ps(a, b)); // accumulate the product
}
alignas(32) float result[8];
_mm256_store_ps(result, sum); // store the accumulated result into an array

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When n == 0, the store instruction (e.g. _mm256_store_ps) still occur in the avx512, avx2, and sse operators that may be harmful to the performance. Is it possible to add a judgment to skip these store instructions?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yeah, I have fixed it

} else if (metric == vsag::MetricType::METRIC_TYPE_L2SQR) {
gt = L2Sqr(data + idx1 * dim, query + i * dim, &dim);
}
REQUIRE(std::abs(gt - value) < 1e-4);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fabs?

@inabao inabao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@LHT129
LHT129 force-pushed the lht_dev branch 7 times, most recently from d5e42ef to 0e8c538 Compare September 14, 2024 07:14

@wxyucs wxyucs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

Comment thread src/io/basic_io.h

namespace vsag {

template <typename IOTmpl>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

IOTmpl need any interface limit ?

Comment thread src/io/memory_io.h
return ret;
}
void
MemoryIO::PrefetchImpl(uint64_t offset, uint64_t cacheLine) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

explicit mark cacheLine to unused ?
cacheLine -> cache_line

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

code style will be modified in following pull request


namespace vsag {

template <MetricType Metric = MetricType::METRIC_TYPE_L2SQR>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Metric -> metric

Comment thread src/io/basic_io.h
}

inline void
Prefetch(uint64_t offset, uint64_t cacheLine = 64) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

cacheLine -> cache_line

Comment thread src/io/memory_io.h
MultiReadImpl(uint8_t* datas, uint64_t* sizes, uint64_t* offsets, uint64_t count) const;

inline void
PrefetchImpl(uint64_t offset, uint64_t cacheLine = 64);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

cacheLine -> cache_line


uint64_t codeSize_{0};

bool isTrained_{false};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

isTrained_ -> is_trained_

private:
uint64_t dim_{0};

uint64_t codeSize_{0};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

codeSize -> code_size_

} else if (Metric == MetricType::METRIC_TYPE_COSINE) {
return InnerProduct(codes1, codes2, &this->dim_); // TODO
} else {
return 0.;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

0.0 ?

@wxyucs wxyucs added the kind/improvement Optimizations, UX polish, or minor improvements 性能优化、体验打磨或细节改良 label Sep 18, 2024
Signed-off-by: LHT129 <tianlan.lht@antgroup.com>
@LHT129
LHT129 merged commit 33755e1 into main Sep 23, 2024
@LHT129
LHT129 deleted the lht_dev branch September 23, 2024 05:56
Roxanne0321 pushed a commit to Roxanne0321/vsag that referenced this pull request Mar 3, 2025
LHT129 added a commit that referenced this pull request May 11, 2026
Signed-off-by: LHT129 <tianlan.lht@antgroup.com>
Sia-Sheerland pushed a commit to Sia-Sheerland/vsag that referenced this pull request Jun 26, 2026
…up#25)

Signed-off-by: LHT129 <tianlan.lht@antgroup.com>
Signed-off-by: Sia Sheerland <x1075956441x@163.com>

Signed-off-by: Sia Sheerland <x1075956441x@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/improvement Optimizations, UX polish, or minor improvements 性能优化、体验打磨或细节改良

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants