Improving relay compile error diagnostics

#1

I’ve been working on a custom Relay transformation pass and have had a great deal of difficulty debugging, because type inference will hit errors, but not tell me where they are in the source module; all I get is a stack trace.

I noticed that there’s a notion of CompileError instead of just regular Error / the ICHECK macros. When I use CompileErrors, I get a nice listing of multiple errors… still without span information, since the Relay module doesn’t have that information populated. (It’s created from an ONNX graph.)

I’ve been wondering about a few things.

  1. Would there be interest in replacing the ICHECK macros with something like CCHECK macros that throw compile errors instead of regular errors? These could be used in e.g. the type relation checking code in Relay. (ICHECK would still be used in most places ofc, just wherever compile errors make sense.)

  2. Would it be possible to automatically create span information for a Relay graph using the text format? I tried serializing the module and then parsing it back to populate the spans; unfortunately this doesn’t work, because some of my local variables have "."s in their names, which the parser can’t handle. I’d also be interested in fixing that unless there are objections.

CC @jroesch

#2

Hi @kazimuth, regarding (1), DiagnosticContext::EmitFatal is the correct way to throw compile errors. Ideally all type relations (and any other compilation errors) would use this machinery. But there are a lot of type relations that use ICHECK instead. They should be rewritten, but no one has put in the effort to rewrite them all. It’ll be a bit of work. A macro might make things easier, but there is some difficulty in writing one because you need a DiagnosticContext to report to.

1 Like
#3

Maybe a macro involving a context would work. e.g.

CCHECK(ctx, a == b) << "help the computer is on fire";

#4

Sorry for the slow response, lots of stuff going lately and I was also sick last week.

@kazimuth There is actually a pass which already does exactly what you suggested I think the problem is there is not currently an “easy” way to turn on in the default flow. It should be pretty easy to come up with a design for enabling this in the compiler flow, a few people complained that they didn’t want this on by default.

See: python/tvm/relay/transform/transform.py

def AnnotateSpans():
    """
    Annotate a program with span information by first generating its textual
    representation and then parsing it back into a Relay AST annotated with
    span information.

    Returns
    -------
    ret : tvm.transform.Pass
        The registered AnnotateSpans pass.
    """
    return _ffi_api.AnnotateSpans()

Another option is to use the “null” Spans to re-enable the old style of error reporting where we print out the entire program and annotate it with errors.

Happy to talk more on Discord if you want to chat more intently about this.