Closed kayaercument closed 1 month ago
Guess we should be exposing that somehow in that case. Computing the size isn't hard on our side. Storing it in the fragment should also work just fine. Anything I am overlooking?
We are considering to submitting the circuit like this. I think either we can change the submit function and submit the size with the IRs, or we can create another function for it.
We are considering to submitting the circuit like this. I think either we can change the submit function and submit the size with the IRs, or we can create another function for it.
How about embedding the information in the QDMI_Fragment and simply adding a size_t QDMI_Fragment_size(QDMI_Fragment* frag)
function for querying that information?
I don't think the backends would need to query that information. It can use something like this:
int QDMI_control_submit(QDMI_Device dev, QDMI_Fragment *frag, int numshots, QInfo info, QDMI_Job *job){
...
size_t frag_size = (*frag)->sizebuffer;
...
The problem is how the submitter will pack this information. I suggest we change the packing function like this:
int QDMI_control_pack_qir(QDMI_Device dev, void *qirmod, QDMI_Fragment *frag, size_t size)
The other option would be adding another function like this:
int QDMI_control_pack_size(QDMI_Device dev, QDMI_Fragment *frag, size_t size)
The backends might not have to query it, but the submitter (or, specifically, the QDMI_control_submit
method) obviously does.
QDMI_Fragment
is an opaque pointer and I believe we shouldn't be accessing internal details of the struct without going through an API (like I proposed above) for accessing these quantities.
As for changing the signature of the packing routines: what we essentially need is kind of a constructor for a QDMI_Fragment
that takes in all the necessary information and produces the corresponding information.
If the size
is a required parameter for that constructor, it should be added as a parameter of the corresponding function.
The question we could additionally think about is whether we would actually want to rename the QDMI_control_pack_size
method to QDMI_Fragment_create
(to reflect that it's constructing the fragment).
I would also guess that this is a method that not every backend implementation needs to provide, but that is provided by the QDMI core library.
I believe a create function for QDMI_Fragment
can be redundant. I would suggest something like this:
Submitter side:
void *qirmod = static_cast<void *>(buffer.data());
size_t modsize = buffer.size()
err = QDMI_control_pack_qir(this->mDevice, &frag, qirmod, size);
if(err == QDMI_ERROR_NOTSUPPORTED){
char* qasm2 = toQasm(module);
err = QDMI_control_pack_qasm2(this->mDevice, &frag, qasm2, strlen(qasm2));
...
}
Backend side:
int QDMI_control_pack_qasm2(QDMI_Device dev, QDMI_Fragment *frag, char *qasmstr, size_t size)
{
(*frag) = malloc(sizeof(QDMI_Fragment_t));
(*frag)->qasmstr = malloc(size);
strcpy((*frag)->qasmstr, qasmstr);
}
int QDMI_control_pack_qir(QDMI_Device dev, QDMI_Fragment *frag, void *qirmod, size_t size)
{
(*frag) = malloc(sizeof(QDMI_Fragment_t));
(*frag)->qirmod = qirmod;
...
}
So in my opinion this doesn't work until you expose functions for instance destruction in the API. Where do you free the QDMI_Fragment_t otherwise ? ( see also this discussion https://github.com/Munich-Quantum-Software-Stack/QDMI/issues/52)
So in my opinion this doesn't work until you expose functions for instance destruction in the API.
Where do you free the QDMI_Fragment_t otherwise ? ( see also this discussion https://github.com/Munich-Quantum-Software-Stack/QDMI/issues/52)
Yeah. Fully agree with that. Quite some of the opaque data structures throughout the current library need a couple more utility functions.
I'll be on my way to QCE tomorrow and plan to dedicate some time of that to write down a request for comment on how to adapt parts of QDMI, so that they become a little more usable and so that we can avoid leaking memory left and right 😌 I'll probably post the result as a discussion here in this repository.
@burgholzer Is there any update on this?
@burgholzer Is there any update on this?
@ystade and I are working on a general update for QDMI that should address this issue. It might take us a little while to get that ready. We will try to keep you posted.
Please keep me and @echavarria-lrz posted and create discussions/issues so we all can be on the same page.
The vendors are reporting to us that they need the size of the IR. They can work around it if QASM is packed. However, they would need the size of the QIR code. Even though we have
sizebuffer
in the QDMI_Frag, we don't have a function to pack it.FYI @echavarria-lrz