Implement gemini.logical and draft kernel validation #558
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
| # TODO: this is actually tricky, since qalloc calls qubit.new multiple times and we have to make sure qalloc is only called once | ||
| # but it can technically contain many qubit.new calls | ||
| # if interp.has_allocated_qubits: | ||
| # raise ir.ValidationError( | ||
| # stmt, "Can only allocate qubits once in a logical Gemini program!" | ||
| # ) | ||
|
|
||
| # interp.has_allocated_qubits = True |
There was a problem hiding this comment.
Actually what matters is if qubit are allocated after the first gate so maybe add a flag for if there ways already a gate operation
There was a problem hiding this comment.
Actually I'm not even sure if its a problem if there are allocations after the first gate.
There was a problem hiding this comment.
Wouldn't it be sufficient for Gemini to ensure that at most n qubits are allocated? In principle it would not make a difference where in the squin kernel qalloc is called, would it? On the machine, all qubits are always prepared at the very beginning.
There was a problem hiding this comment.
You're right. The only problem here would be conditional allocation. But since the dialect won't support if statements, we can lift basically all qubit allocations to the top of the kernel.
The only case that we have to think about are for loops, which are allowed, but restricted to ones that can be unrolled. I'm not sure if the AddressAnalysis can infer the number of qubits in a for loop, however. Technically, since we only support for loops that can be unrolled, it is inferable, but we might just require the user (or previous passes) to unroll before the validation is run. So either
- make sure the number of qubits is properly inferred in simple (constant iterable) for loops, or
- error (or warn?) for allocations inside for loops, telling the user to unroll it. This could be done via a boolean flag in the
logicaldialect group, running theAggressiveUnroll(I think we need to inline anyway).
@weinbe58 yes, you're right this should be an abstract class. Originally, I was thinking we'd potentially want to share methods using this single validation analysis (similar to how we only have a single PyQrack interpreter), but since we need to override (or remove) methods at times, this doesn't make much sense. So, the base class will basically be a template that defines the typing and the lattice along with the single default fallback method of having |
|
While implementing a validation that checks whether
Since I'm not sure if this is actually an issue on the kirin side of things. @weinbe58 what do you think? |
Well since you want to indicate which qubit has the invalid initialization storing the error at the qubit ssa value probably makes more sense anyways. |
@weinbe58 I'm not so sure. Ideally, I'd like to point to the (invalid) gate statement, not the qubit that is allocated somewhere. Also, how would you deal with multiple errors for a single qubit then? Generally speaking, I think we should be able to express errors (or any analysis value, really), even for statements that don't have a |
Well you can store the invalid statements in a custom frame. that is associated with the Analysis. This is similar to how purity is handled in constant prop. |
|
So, after discussion with @weinbe58 elsewhere: the values in the Instead, we now just append every error we encounter as an |
|
@weinbe58 I'd actually like to conclude this RFC and merge it. In that sense, this is ready for review. I think the validation for the logical dialect is complete, except for the qubit number validation. However, that requires the address analysis to work, so it's blocked by that. So I'd do that in a separate PR, if you agree. |
This implements the simple dialect group as discussed in #531.
However, it also contains a draft (RFC) for validating kernels, using the above as an example.
I thought a bit about it and in the end decided to split the validation into two parts:
ValidationAnalysispass with an appropriate lattice (could be extended to warnings, etc.) that checks for errors in the IR.KernelValidationclass, that gathers up the errors and throws them askirin.ir.ValidationErrorwith the appropriate source info pointing to the line of the error. I went back and forth as to how to implement this, since it is a bit similar to interpretation. However, in the end I just decided to go with a very simple dataclass, since it is sufficiently different from an interpreter or a rewrite.One thing that's not great and I'm not sure how to improve: we probably don't want the
ValidationAnalysisto implement a lot of methods via method tables. For example, thegemini.logicaldialect doesn't supportscf.IfElse. However, if we want to re-use theValidationAnalysiselsewhere, then registering aMethodTableforscfwith a method forscf.IfElsemight fail validation in kernels where this should be fine. At the same time, we might want to share validation between different kernels.The way I "solved" this for now is to define a new analysis pass just for the logical dialect that inherits from the generic
ValidationAnalysis. If we go this route, then theValidationAnalysisbasically just defines the lattice and a fallback method.There's also some TODOs left here, e.g. how to display multiple errors, but I thought I'd get comments as soon as possible.