Skip to content

Commit

Permalink
Remove some unnecessary TODOs.
Browse files Browse the repository at this point in the history
  • Loading branch information
@martun
martun committed Aug 5, 2024
1 parent 7bd38cb commit 8ab7344
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 87 deletions.
8 changes: 0 additions & 8 deletions libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,12 @@ namespace nil {
}

constexpr element_fp &operator-=(const element_fp &B) {
// TODO(martun): consider directly taking the backend and calling
// eval_add to improve performance.
data -= B.data;

return *this;
}

constexpr element_fp &operator+=(const element_fp &B) {
// TODO(martun): consider directly taking the backend and calling
// eval_add to improve performance.
data += B.data;

return *this;
Expand Down Expand Up @@ -224,10 +220,6 @@ namespace nil {
return element_fp(inverse_mod(data));
}

// TODO: complete method
constexpr element_fp _2z_add_3x() {
}

constexpr element_fp squared() const {
return element_fp(data * data); // maybe can be done more effective
}
Expand Down
2 changes: 1 addition & 1 deletion libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp12_2over3over2.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ namespace nil {

element_fp12_2over3over2 sqrt() const {

// compute squared root with Tonelli--Shanks
// TODO: compute squared root with Tonelli--Shanks
}

element_fp12_2over3over2 squared() const {
Expand Down
14 changes: 2 additions & 12 deletions libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp2.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,6 @@ namespace nil {
}

constexpr element_fp2 operator*(const element_fp2 &B) const {
// TODO: the use of data and B.data directly in return statement addition cause constexpr
// error for gcc
const underlying_type A0 = data[0], A1 = data[1], B0 = B.data[0], B1 = B.data[1];
const underlying_type A0B0 = data[0] * B.data[0], A1B1 = data[1] * B.data[1];

Expand Down Expand Up @@ -185,10 +183,6 @@ namespace nil {
return element_fp2(-(data[1] * y_b), data[0] * y_b);
}

constexpr element_fp2 _2z_add_3x() {
return element_fp2(data[0]._2z_add_3x(), data[1]._2z_add_3x());
}

constexpr element_fp2 divBy2() const {
return element_fp2(divBy2(data[0]), divBy2(data[1]));
}
Expand Down Expand Up @@ -245,12 +239,9 @@ namespace nil {
}

constexpr element_fp2 squared() const {
// return (*this) * (*this); // maybe can be done more effective

/* Devegili OhEig Scott Dahab --- Multiplication and Squaring on Pairing-Friendly
* Fields.pdf; Section 3 (Complex squaring) */
// TODO: reference here could cause error in constexpr for gcc
const underlying_type A = data[0], B = data[1];
const underlying_type& A = data[0], B = data[1];
const underlying_type AB = A * B;

return element_fp2((A + B) * (A + non_residue * B) - AB - non_residue * AB, AB + AB);
Expand All @@ -261,8 +252,7 @@ namespace nil {

/* Devegili OhEig Scott Dahab --- Multiplication and Squaring on Pairing-Friendly
* Fields.pdf; Section 3 (Complex squaring) */
// TODO: reference here could cause error in constexpr for gcc
const underlying_type A = data[0], B = data[1];
const underlying_type& A = data[0], B = data[1];
const underlying_type AB = A * B;

data[0] = (A + B) * (A + non_residue * B) - AB - non_residue * AB;
Expand Down
129 changes: 66 additions & 63 deletions libs/algebra/test/bench_test/bench_fields.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,42 @@ using namespace nil::crypto3::algebra;

BOOST_AUTO_TEST_SUITE(fields_manual_tests)

template<class ValueType>
std::vector<std::chrono::duration<double, std::nano>> gather_stats(
std::vector<ValueType>& points1,
std::vector<ValueType>& points2,
std::function<void(std::vector<ValueType>& result,
const std::vector<ValueType>& samples,
std::size_t sample)> operation,
size_t samples_per_batch,
const std::string& operation_name,
size_t BATCHES = 1000) {
using duration = std::chrono::duration<double, std::nano>;

std::vector<duration> batch_duration(BATCHES);
auto save = points1[3];

for(size_t b = 0; b < BATCHES; ++b) {
auto start = std::chrono::high_resolution_clock::now();
for(size_t i = 0; i < samples_per_batch; ++i) {
operation(points1, points2, i);
}

auto finish = std::chrono::high_resolution_clock::now();
batch_duration[b] = (finish - start) * 1.0 / samples_per_batch;
}

// prevent value 'result' from optimizating out
std::cerr << save << std::endl;

auto average_duration = std::accumulate(batch_duration.begin(), batch_duration.end(), duration(0)) / batch_duration.size();
std::cout << "Average time for operator " << operation_name << ": "
<< std::fixed << std::setprecision(3) << average_duration.count() / samples_per_batch
<< " ns" << std::endl;

return batch_duration;
};

template<class Field>
void run_perf_test(std::string const& field_name) {
using namespace nil::crypto3;
Expand All @@ -71,93 +107,61 @@ void run_perf_test(std::string const& field_name) {
// size of arrays is 4 times larger than typical L1 data cache
size_t SAMPLES_COUNT = 4*32*1024/sizeof(value_type);

for (int i = 0; i < SAMPLES_COUNT; ++i) {
for (std::size_t i = 0; i < SAMPLES_COUNT; ++i) {
points1.push_back(algebra::random_element<Field>());
points2.push_back(algebra::random_element<Field>());
}

auto gather_stats = [&points1, &points2]
(std::function<void(std::vector<value_type> & result, std::vector<value_type> const& samples, std::size_t sample)> operation,
size_t samples_per_batch, const std::string& operation_name) {
size_t BATCHES = 1000;

using duration = std::chrono::duration<double, std::nano>;

std::vector<duration> batch_duration;
batch_duration.resize(BATCHES);
auto save = points1[3];;

for(size_t b = 0; b < BATCHES; ++b) {
// if (b % (BATCHES/10) == 0) std::cerr << "Batch progress:" << b << std::endl;
auto start = std::chrono::high_resolution_clock::now();
auto points_index = 0;

for(size_t i = 0; i < samples_per_batch; ++i) {
operation(points1, points2, i);
++points_index;
if (points_index == 1000)
points_index = 0;
}
const size_t BATCHES = 1000;

auto finish = std::chrono::high_resolution_clock::now();
batch_duration[b] = (finish - start) * 1.0 / samples_per_batch;
}

// prevent value 'result' from optimizating out
std::cerr << save << std::endl;

auto s = batch_duration[0];
for(size_t b = 1; b < batch_duration.size(); ++b) {
s += batch_duration[b];
}

s /= batch_duration.size() - 2;
std::cout << "Average time for operator " << operation_name << ": " << std::fixed << std::setprecision(3) << s.count() << std::endl;

return batch_duration;
};


for(int mult = 1; mult <= 100; ++mult) {
int MULTIPLICATOR = mult;
std::cout << "MULT: " << MULTIPLICATOR << std::endl;

auto plus_results = gather_stats(
int mult = 100;
auto plus_results = gather_stats<value_type>(
points1,
points2,
[&](std::vector<value_type> &result, std::vector<value_type> const& samples, std::size_t sample)
{
for(int m = 0; m < MULTIPLICATOR; m++)
for(int m = 0; m < mult; m++)
result[sample*(sample+m) % SAMPLES_COUNT] += samples[sample*(sample+m)*17 % SAMPLES_COUNT];
}, 10000 / MULTIPLICATOR, "Addition");
}, 10000 / mult, "Addition", BATCHES);

auto mul_results = gather_stats(
auto mul_results = gather_stats<value_type>(
points1,
points2,
[&](std::vector<value_type> &result, std::vector<value_type> const& samples, std::size_t sample)
{
for(int m = 0; m < MULTIPLICATOR; m++)
for(int m = 0; m < mult; m++)
result[sample*(sample+m) % SAMPLES_COUNT] *= samples[sample*(sample+m)*17 % SAMPLES_COUNT];
}, 1000 / MULTIPLICATOR, "Multiplication");
}, 1000 / mult, "Multiplication", BATCHES);

auto minus_results = gather_stats(
auto minus_results = gather_stats<value_type>(
points1,
points2,
[&](std::vector<value_type> &result, std::vector<value_type> const& samples, std::size_t sample)
{
for(int m = 0; m < MULTIPLICATOR; m++)
for(int m = 0; m < mult; m++)
result[sample*(sample+m) % SAMPLES_COUNT] -= samples[sample*(sample+m)*17 % SAMPLES_COUNT];
}, 10000 / MULTIPLICATOR, "Subtraction");
}, 10000 / mult, "Subtraction", BATCHES);

auto sqr_results = gather_stats(
auto sqr_results = gather_stats<value_type>(
points1,
points2,
[&](std::vector<value_type> &result, std::vector<value_type> const& samples, std::size_t sample)
{
for(int m = 0; m < MULTIPLICATOR; m++)
for(int m = 0; m < mult; m++)
result[sample*(sample+m) % SAMPLES_COUNT].square_inplace();
}, 1000 / MULTIPLICATOR, "Square In-Place");
}, 1000 / mult, "Square In-Place", BATCHES);

auto inv_results = gather_stats(
auto inv_results = gather_stats<value_type>(
points1,
points2,
[&](std::vector<value_type> &result, std::vector<value_type> const& samples, std::size_t sample)
{
for(int m = 0; m < MULTIPLICATOR; m++)
for(int m = 0; m < mult; m++)
result[sample*(sample+m) % SAMPLES_COUNT] = samples[sample*(sample+m)*17 % SAMPLES_COUNT].inversed();
}, 100 / MULTIPLICATOR, "Inverse");
}, 100 / mult, "Inverse", BATCHES);

char filename[200]= {0};
sprintf(filename,"%s-stats-%03d.csv", field_name.c_str(), MULTIPLICATOR);
sprintf(filename,"%s-stats-%03d.csv", field_name.c_str(), mult);

std::ofstream f(filename, std::ofstream::out);
f << "# " << typeid(Field).name() << std::endl;
Expand All @@ -173,7 +177,6 @@ void run_perf_test(std::string const& field_name) {
}

f.close();
}
}

BOOST_AUTO_TEST_CASE(field_operation_perf_test_pallas) {
Expand Down
1 change: 0 additions & 1 deletion libs/math/include/nil/crypto3/math/polynomial/polynomial_dfs.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ namespace nil {
BOOST_ASSERT_MSG(val.size() == detail::power_of_two(val.size()),
"DFS optimal polynomial size must be a power of two");
}
// TODO: add constructor with omega

polynomial_dfs(polynomial_dfs&& x)
BOOST_NOEXCEPT(std::is_nothrow_move_constructible<allocator_type>::value)
Expand Down
1 change: 0 additions & 1 deletion libs/multiprecision/include/nil/crypto3/multiprecision/modular/modular_adaptor.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ namespace boost {
return eval_is_zero(val.base_data());
}

// TODO: check returned value
template<class Backend, typename ModularParamsType>
BOOST_MP_CXX14_CONSTEXPR int eval_get_sign(const modular_adaptor<Backend, ModularParamsType> &) {
return 1;
Expand Down
1 change: 0 additions & 1 deletion libs/multiprecision/test/modular_adaptor_fixed.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ constexpr void pow_test(const boost::multiprecision::number<Backend, ExpressionT
BOOST_ASSERT_MSG(standard_number(a_m_powm_b.backend().convert_to_cpp_int()) == a_powm_b, "powm error");
}

// TODO: test_set is not ref because of constexpr error in gcc-10
// This test case uses normal boost::cpp_int for comparison to our modular_adaptor with cpp_int_modular_backend.
template<typename Number>
bool base_operations_test(std::array<Number, test_set_len> test_set) {
Expand Down

0 comments on commit 8ab7344

Please sign in to comment.