Specify output schemas #50
Conversation
|
Hi @swernli, @bettinaheim, could you review this PR? |
| METADATA\trequired_num_qubits\t5 | ||
| METADATA\trequired_num_results\t5 | ||
| METADATA\tqir_profiles\tbase_profile | ||
| METADATA\toutput_labeling_format\tformat_id |
There was a problem hiding this comment.
Is format_id here and in the QIR above (also on other files) meant to be a placeholder for the actual ID?
There was a problem hiding this comment.
I believe it is, but I am not 100% sure what the values look like. Previously it was called schema_id and was present in the base profile for the attribute "output_labeling_schema", see https://github.com/qir-alliance/qir-spec/blob/main/specification/under_development/profiles/Base_Profile.md#attributes
If I were to make a guess, the values should be "ordered" and "labeled".
If that sounds good to you, I will update the examples to reflect that.
There was a problem hiding this comment.
I think that makes sense. Either "ordered" or "labeled" seem appropriate here.
There was a problem hiding this comment.
@swernli actually, I just realized that that distinction is covered by the schema_name HEADER so putting it under output_labeling_schema is redundant.
Since the profiles specify the attribute "output_labeling_{schema,format}", we would like to match it with the metadata in the output. We have two options:
- Eliminate
output_labeling_formatfrom the METADATA output and keep the original attribute"output_labeling_schema"and relate it to theschema_nameHEADER, OR - Figure out the intended use for
output_labeling_formatand specify what can be used forformat_id. This will also require either introducing a new attribute to specify the output schema in the profiles or inferring it from the output function signatures.
I prefer option 1 for its simplicity.
@bettinaheim do you happen to have an insight on the history of the attribute "output_labeling_schema"? In case we are missing something, and it is option 2 that we should focus on.
There was a problem hiding this comment.
As raised by the steering committee, the second option being a breaking change, I went with option 1.
also add a link to output records as suggested by Stefan
cspell.json
Outdated
There was a problem hiding this comment.
Added this file for the commonly occurring (~ >=5 occurrences) technical words. Helps reduce the noise in the diff and also to catch real mistakes.
| @@ -170,7 +171,7 @@ declare void @__quantum__rt__result_record_output(%Result*, i8*) | |||
|
|
|||
| ; attributes | |||
|
|
|||
| attributes #0 = { "entry_point" "qir_profiles"="base_profile" "output_labeling_schema"="schema_id" "required_num_qubits"="2" "required_num_results"="2" } | |||
| attributes #0 = { "entry_point" "qir_profiles"="base_profile" "output_labeling_format"="format_id" "required_num_qubits"="2" "required_num_results"="2" } | |||
There was a problem hiding this comment.
It came up during steering committee meeting that this would technically be a breaking change, since the attribute name is different. Could you revert this part of the change to keep the name output_labeling_schema here and in the text sections?
| @@ -675,14 +675,14 @@ information about the `switch` instruction. | |||
|
|
|||
| The following runtime functions must be supported by all backends: | |||
|
|
|||
| | Function | Signature | Description | | |||
| | :---------------------------------- | :------------------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | |||
There was a problem hiding this comment.
From the steering committee: could you revert the formatting fixes in this and the other profile? There was a preference to avoid formatting changes here and keep the updates to the profile to be only functional changes to text or links.
Also rename schema_name to schema_id
This reverts part of the commit bb3af1f.
Fork of and supersedes #36, the diff between the two PRs can be seen at cesarzc/qir-spec@output-schemas...qartik:qir-spec:output-schemas
Or if you want to avoid looking at changes already in
main, see qartik/qir-spec@4146608...output-schemas