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

chore: print warning in builder when failure happens. #11205

Merged
merged 5 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 1 addition & 2 deletions barretenberg/cpp/src/barretenberg/bb/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1314,8 +1314,7 @@ int main(int argc, char* argv[])
{
try {
std::vector<std::string> args(argv + 1, argv + argc);
debug_logging = flag_present(args, "-d") || flag_present(args, "--debug_logging");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

deleted because it was not being used anywhere. Instead use NDEBUG env var.

verbose_logging = debug_logging || flag_present(args, "-v") || flag_present(args, "--verbose_logging");
verbose_logging = flag_present(args, "-v") || flag_present(args, "--verbose_logging");
if (args.empty()) {
std::cerr << "No command provided.\n";
return 1;
Expand Down
3 changes: 0 additions & 3 deletions barretenberg/cpp/src/barretenberg/common/log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,3 @@ bool verbose_logging = std::getenv("BB_VERBOSE") == nullptr ? false : std::strin
#else
bool verbose_logging = true;
#endif

// Used for `debug` in log.hpp.
bool debug_logging = false;
5 changes: 1 addition & 4 deletions barretenberg/cpp/src/barretenberg/common/log.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,10 @@ template <typename... Args> std::string benchmark_format(Args... args)
return os.str();
}

extern bool debug_logging;
#ifndef NDEBUG
template <typename... Args> inline void debug(Args... args)
{
if (debug_logging) {
logstr(format(args...).c_str());
}
logstr(format(args...).c_str());
}
#else
template <typename... Args> inline void debug(Args... /*unused*/) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ template <typename FF_> class CircuitBuilderBase {
using EmbeddedCurve = std::conditional_t<std::same_as<FF, bb::g1::coordinate_field>, curve::BN254, curve::Grumpkin>;

size_t num_gates = 0;
// true if we have dummy witnesses (in the write_vk case)
bool has_dummy_witnesses = false;

std::vector<uint32_t> public_inputs;
std::vector<FF> variables;
Expand Down Expand Up @@ -56,7 +58,7 @@ template <typename FF_> class CircuitBuilderBase {
static constexpr uint32_t REAL_VARIABLE = UINT32_MAX - 1;
static constexpr uint32_t FIRST_VARIABLE_IN_CLASS = UINT32_MAX - 2;

CircuitBuilderBase(size_t size_hint = 0);
CircuitBuilderBase(size_t size_hint = 0, bool has_dummy_witnesses = false);

CircuitBuilderBase(const CircuitBuilderBase& other) = default;
CircuitBuilderBase(CircuitBuilderBase&& other) noexcept = default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
#include "circuit_builder_base.hpp"

namespace bb {
template <typename FF_> CircuitBuilderBase<FF_>::CircuitBuilderBase(size_t size_hint)
template <typename FF_>
CircuitBuilderBase<FF_>::CircuitBuilderBase(size_t size_hint, bool has_dummy_witnesses)
: has_dummy_witnesses(has_dummy_witnesses)
{
variables.reserve(size_hint * 3);
variable_names.reserve(size_hint * 3);
Expand Down Expand Up @@ -286,6 +288,10 @@ template <typename FF_> void CircuitBuilderBase<FF_>::set_err(std::string msg)

template <typename FF_> void CircuitBuilderBase<FF_>::failure(std::string msg)
{
if (!has_dummy_witnesses) {
// We have a builder failure when we have real witnesses which is a mistake.
info("Builder failure when we have real witnesses!"); // not a catch-all error
}
_failed = true;
set_err(std::move(msg));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase<typename ExecutionTrace_:
const std::vector<uint32_t>& public_inputs,
size_t varnum,
bool recursive = false)
: CircuitBuilderBase<FF>(size_hint)
: CircuitBuilderBase<FF>(size_hint, witness_values.empty())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

both mega and ultra circuit builders will call this, so it should account for all write_vk flows

{
// TODO(https://github.com/AztecProtocol/barretenberg/issues/870): reserve space in blocks here somehow?

Expand Down
Loading