feat(eval): polynomial value inspection in debug mode

[chaosma]

Issue Link / Motivation Sometimes we need to inspect the values of custom gate/polynomial over a specific row

Changes Overview Added inspection flag, when use inspect=true, will print out values of a given row in debug mode.

[drouyang]

Adding debug flag to functions is anti-pattern. We should stop doing that.

Typically, we should have a per-module/file debug flag that can be set at compile time to enable and disable debug prints.

[drouyang]

@cyphersnake how can we add per-module compile-time debug flag?

I've never seen a debug flag as a function parameter in complex system software. Debug is a compile-time setting instead of a runtime parameter...

[drouyang]

Consider adding debug feature in Cargo?

[drouyang]

Or consider a logging facility that can log as DEBUG or INFO, which allows us to tweak verbose level at runtime.

Some kind of self-built or third-party debug logging facility, instead of ad-hoc prints

[cyphersnake]

@drouyang We don't need any special mechanisms, we just need to stop being afraid of spamming logs at the DEBUG level. As long as the test case does not take up 10 GB, everything is fine. We have almost no logs at the TRACE level and above.

[chaosma]

Please don't add runtime checks to display logs. I will repeat again that the DEBUG level exists only for the sake of debugging. It is akin to println, which can be left for later. Moreover, even for debugging, you can customise it per module

https://github.com/snarkify/sirius/blob/main/src/ivc/incrementally_verifiable_computation.rs#L240

This is runtime check, although it happens not often.

One work around is:

#[cfg(debug_assertions)]
    {
        // Using `option_env!` to optionally get the row number from an environment variable.
        // `DEBUG_EVAL_ROW` is the name of the environment variable.
        if let Some(debug_row_str) = option_env!("DEBUG_EVAL_ROW") {
            if let Ok(debug_row) = debug_row_str.parse::<usize>() {
                if row == debug_row {
                    debug!(
                        "Polynomial Value Inspection, row = {}, col={:?}, val = {:?}",
                        row, &column_index, &v1
                    );
                }
            }
        }
    }

Is there any better approach? @cyphersnake

[cyphersnake]

I don't understand what problem you want to solve. Do you find it difficult to work with excessive logs? I can help with tuling with this problem.

If you have any calculations that should only be within the logs, then just stick them inside the debug! macro as the expression debug!("message {}", { expr }); and it will only execute if the DEBUG log level is enabled

[chaosma]

@drouyang I usually didn't add debug code. For this PR, I just follow the style in #194:

https://github.com/snarkify/sirius/blob/main/src/ivc/incrementally_verifiable_computation.rs#L240

I am okay with or without the debug mode. We just need to make an agreement on the style.

[chaosma]

I don't understand what problem you want to solve. Do you find it difficult to work with excessive logs? I can help with tuling with this problem.

If you have any calculations that should only be within the logs, then just stick them inside the debug! macro as the expression debug!("message {}", { expr }); and it will only execute if the DEBUG log level is enabled

@cyphersnake I want to print out debug log for a specific row when needed: data.eval(poly, row0, true) instead of print all rows in debug mode. my change is similar to #194

[cyphersnake]

I see your problem.

This is usually done by using log scopes. I've wanted to introduce them for a long time, but I've been postponing this moment. I will migrate to tracing of slog, then you can mark all logs inside a certain function with a some prefix

[cyphersnake]

After #198, you can just debug! all time, but add at one level above code:

let my_span = span!(Level::TRACE, "this is context of usage of eval, what I need at the final logs");
let _enter = my_span.enter();

And during the _enter lifetime all logs will be marked with this prefix, by which you can grep.

I repeat that if there are a lot of logs, it is not a big deal, as long as ripgrep does not break, the main thing is to search them effectively. I also note that you can include for example DEBUG level only on the module you are interested in, and leave INFO on the others

One of my first projects every generated 17 GB of logs per one test, it was a pain, however, more than once it accelerated debugging, and thanks to fast search - did not slow down work in simple cases

[drouyang]

@drouyang I usually didn't add debug code. For this PR, I just follow the style in #194:

https://github.com/snarkify/sirius/blob/main/src/ivc/incrementally_verifiable_computation.rs#L240

I am okay with or without the debug mode. We just need to make an agreement on the style.

I should have rejected #194. Will follow up.

I would like to propose that

• we should avoid adding runtime debug checks.
• it's good practice to add a lot of debug and trace logs, as long as they can be controlled via verbose level and does not introduce overhead in a release build
• for logic that only runs in debug mode, use compile-time DEBUG feature?

The goal of this PR is more complicated: debug print with runtime condition check. Typically I don't commit this kind of debug prints because of it's complexity. Mikhail's grep-based approach is one way to mitigate it. For simplicity, maybe just keep it local?

[cyphersnake]

@chaosma do you understand my idea with spans for debug? Do you need my help with that?

[chaosma]

@chaosma do you understand my idea with spans for debug? Do you need my help with that?

No, I don't understand your idea of spans for debug yet. @cyphersnake

[chaosma]

@drouyang I usually didn't add debug code. For this PR, I just follow the style in #194: https://github.com/snarkify/sirius/blob/main/src/ivc/incrementally_verifiable_computation.rs#L240 I am okay with or without the debug mode. We just need to make an agreement on the style.

I should have rejected #194. Will follow up.

I would like to propose that

• we should avoid adding runtime debug checks.
• it's good practice to add a lot of debug and trace logs, as long as they can be controlled via verbose level and does not introduce overhead in a release build
• for logic that only runs in debug mode, use compile-time DEBUG feature?

The goal of this PR is more complicated: debug print with runtime condition check. Typically I don't commit this kind of debug prints because of it's complexity. Mikhail's grep-based approach is one way to mitigate it. For simplicity, maybe just keep it local?

Let's have a discussion next week about this and #194. It makes code cleaner to avoid additional debug flags in the code. But need figure out a way for this kind of debug.