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

refactor: Remove copy from compute_row_evaluations #8875

Merged
merged 6 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions barretenberg/cpp/scripts/compare_branch_vs_baseline_remote.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ HARDWARE_CONCURRENCY=${HARDWARE_CONCURRENCY:-16}
BASELINE_BRANCH="master"
BENCH_TOOLS_DIR="$BUILD_DIR/_deps/benchmark-src/tools"

if [ ! -z "$(git status --untracked-files=no --porcelain)" ]; then
echo "Git status is unclean; the script will not be able to check out $BASELINE_BRANCH."
exit 1
fi

echo -e "\nComparing $BENCHMARK between $BASELINE_BRANCH and current branch:"

# Move above script dir.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ HARDWARE_CONCURRENCY=${HARDWARE_CONCURRENCY:-16}
BASELINE_BRANCH="master"
BENCH_TOOLS_DIR="$BUILD_DIR/_deps/benchmark-src/tools"

if [ ! -z "$(git status --untracked-files=no --porcelain)" ]; then
echo "Git status is unclean; the script will not be able to check out $BASELINE_BRANCH."
exit 1
fi


echo -e "\nComparing $BENCHMARK between $BASELINE_BRANCH and current branch:"

# Move above script dir.
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/scripts/compare_client_ivc_bench.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env bash
set -eu

./scripts/compare_branch_vs_baseline_remote_wasm.sh client_ivc_bench 'Full/6$'
./scripts/compare_branch_vs_baseline_remote.sh client_ivc_bench 'Full/6$'
4 changes: 4 additions & 0 deletions barretenberg/cpp/scripts/compare_client_ivc_bench_wasm.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/usr/bin/env bash
set -eu

./scripts/compare_branch_vs_baseline_remote_wasm.sh client_ivc_bench 'Full/6$'
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ template <class DeciderProvingKeys_> class ProtogalaxyProverInternal {
using RelationUtils = bb::RelationUtils<Flavor>;
using ProverPolynomials = typename Flavor::ProverPolynomials;
using Relations = typename Flavor::Relations;
using AllValues = typename Flavor::AllValues;
using RelationSeparator = typename Flavor::RelationSeparator;
static constexpr size_t NUM_KEYS = DeciderProvingKeys_::NUM;
using UnivariateRelationParametersNoOptimisticSkipping =
Expand Down Expand Up @@ -54,6 +55,28 @@ template <class DeciderProvingKeys_> class ProtogalaxyProverInternal {

static constexpr size_t NUM_SUBRELATIONS = DeciderPKs::NUM_SUBRELATIONS;

inline static FF process_subrelation_evaluations(const RelationEvaluations& evals,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting note: without this inline keyword I was consistently seeing WASM performance slightly worse than baseline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is despite the fact that there is no corresponding inline for the function RelationUtils::scale_and_batch_elements that this replaces.

Copy link
Contributor

Choose a reason for hiding this comment

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

hm, and the logic really hasnt changed except refactored a bit

const std::array<FF, NUM_SUBRELATIONS>& challenges,
FF& linearly_dependent_contribution)
{
FF linearly_independent_contribution{ 0 };
size_t idx = 0;

auto scale_by_challenge_and_accumulate =
[&]<size_t relation_idx, size_t subrelation_idx, typename Element>(Element& element) {
using Relation = typename std::tuple_element_t<relation_idx, Relations>;
const Element contribution = element * challenges[idx];
if (subrelation_is_linearly_independent<Relation, subrelation_idx>()) {
linearly_independent_contribution += contribution;
} else {
linearly_dependent_contribution += contribution;
}
idx++;
};
RelationUtils::apply_to_tuple_of_arrays_elements(scale_by_challenge_and_accumulate, evals);
return linearly_independent_contribution;
}

/**
* @brief Compute the values of the aggregated relation evaluations at each row in the execution trace, representing
* f_i(ω) in the Protogalaxy paper, given the evaluations of all the prover polynomials and \vec{α} (the batching
Expand All @@ -67,40 +90,41 @@ template <class DeciderProvingKeys_> class ProtogalaxyProverInternal {
* linearly dependent subrelation and α_j is its corresponding batching challenge.
*/
static std::vector<FF> compute_row_evaluations(const ProverPolynomials& polynomials,
const RelationSeparator& alpha,
const RelationSeparator& alphas_,
const RelationParameters<FF>& relation_parameters)

{

BB_OP_COUNT_TIME_NAME("ProtogalaxyProver_::compute_row_evaluations");

const size_t polynomial_size = polynomials.get_polynomial_size();
std::vector<FF> full_honk_evaluations(polynomial_size);
std::vector<FF> aggregated_relation_evaluations(polynomial_size);

const std::array<FF, NUM_SUBRELATIONS> alphas = [&alphas_]() {
std::array<FF, NUM_SUBRELATIONS> tmp;
tmp[0] = 1;
std::copy(alphas_.begin(), alphas_.end(), tmp.begin() + 1);
return tmp;
}();

const std::vector<FF> linearly_dependent_contribution_accumulators = parallel_for_heuristic(
polynomial_size,
/*accumulator default*/ FF(0),
[&](size_t row, FF& linearly_dependent_contribution_accumulator) {
auto row_evaluations = polynomials.get_row(row);
RelationEvaluations relation_evaluations;
RelationUtils::zero_elements(relation_evaluations);

RelationUtils::template accumulate_relation_evaluations<>(
row_evaluations, relation_evaluations, relation_parameters, FF(1));

auto output = FF(0);
auto running_challenge = FF(1);
RelationUtils::scale_and_batch_elements(relation_evaluations,
alpha,
running_challenge,
output,
linearly_dependent_contribution_accumulator);

full_honk_evaluations[row] = output;
[&](size_t row_idx, FF& linearly_dependent_contribution_accumulator) {
const AllValues row = polynomials.get_row(row_idx);
// Evaluate all subrelations on the given row. Separator is 1 since we are not summing across rows here.
const RelationEvaluations evals =
RelationUtils::accumulate_relation_evaluations(row, relation_parameters, FF(1));

// Sum against challenges alpha
aggregated_relation_evaluations[row_idx] =
process_subrelation_evaluations(evals, alphas, linearly_dependent_contribution_accumulator);
},
thread_heuristics::ALWAYS_MULTITHREAD);
full_honk_evaluations[0] += sum(linearly_dependent_contribution_accumulators);
return full_honk_evaluations;
}
aggregated_relation_evaluations[0] += sum(linearly_dependent_contribution_accumulators);

return aggregated_relation_evaluations;
}
/**
* @brief Recursively compute the parent nodes of each level in the tree, starting from the leaves. Note that at
* each level, the resulting parent nodes will be polynomials of degree (level+1) because we multiply by an
Expand Down
13 changes: 7 additions & 6 deletions barretenberg/cpp/src/barretenberg/relations/utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,16 @@ template <typename Flavor> class RelationUtils {
*/
template <typename Parameters>
// TODO(#224)(Cody): Input should be an array?
inline static void accumulate_relation_evaluations(const PolynomialEvaluations& evaluations,
RelationEvaluations& relation_evaluations,
const Parameters& relation_parameters,
const FF& partial_evaluation_result)
inline static RelationEvaluations accumulate_relation_evaluations(const PolynomialEvaluations& evaluations,
const Parameters& relation_parameters,
const FF& partial_evaluation_result)
{
RelationEvaluations result;
constexpr_for<0, NUM_RELATIONS, 1>([&]<size_t rel_index>() {
accumulate_single_relation<Parameters, rel_index>(
evaluations, relation_evaluations, relation_parameters, partial_evaluation_result);
evaluations, result, relation_parameters, partial_evaluation_result);
});
return result;
}

template <typename Parameters, size_t relation_idx, bool consider_skipping = true>
Expand Down Expand Up @@ -336,7 +337,7 @@ template <typename Flavor> class RelationUtils {
* dependent contribution when we compute the evaluation of full rel_U(G)H at particular row.)
*/
template <size_t outer_idx = 0, size_t inner_idx = 0, typename Operation, typename... Ts>
static void apply_to_tuple_of_arrays_elements(Operation&& operation, std::tuple<Ts...>& tuple)
static void apply_to_tuple_of_arrays_elements(Operation&& operation, const std::tuple<Ts...>& tuple)
{
using Relation = typename std::tuple_element_t<outer_idx, Relations>;
const auto subrelation_length = Relation::SUBRELATION_PARTIAL_LENGTHS.size();
Expand Down
Loading