Skip to content

Add DisCoCirc tutorial on babi task #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

ouissal-moumou
Copy link

@ouissal-moumou ouissal-moumou commented Feb 11, 2025

This adds a tutorial showing how to use a DisCoCirc model for solving the babi6 task.

This is the first version of the tutorial. Ignore any errors that might stem from previous unsuccessful runs of the experiments.

@dimkart dimkart changed the title First Draft of the Tutorials Add DisCoCirc tutorial on babi task Feb 17, 2025
@dimkart
Copy link
Collaborator

dimkart commented Feb 17, 2025

@ouissal-moumou Are there two separate notebooks? Can we just merge everything into one? If not, we will need a main index file that joins the two parts.

@neiljdo FYI

@neiljdo
Copy link
Collaborator

neiljdo commented Mar 20, 2025

@dimkart it seems the two notebooks can be merged into one to improve the flow of the tutorial - the second notebook is just the training step + result visualization. Doing so also makes this tutorial similar to the existing ones.

Also, the tutorial might benefit from some refactoring - I can work on this incrementally.

@dimkart
Copy link
Collaborator

dimkart commented Mar 20, 2025

@neiljdo Great, and if you think you can merge the two notebooks let's do this.

Copy link

@DNA386 DNA386 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall good start on the tutorials, but there's a few places that could do with simplification to make the tutorial clearer. Some general comments to keep in mind:

  • try to keep your explanatory sentences shorter. some are very verbose, which can make it harder to follow what you mean.
  • in general it is more 'pythonic' to use list/dict comprehensions where you can rather than for loops. (at least in my opinion) this will typically make your code easier to follow too. I've tried to suggest the simplifications in places I think it would help, but feel free to disagree!

"metadata": {},
"source": [
"\n",
"# Tutorial: babI6 Training and Preprocessing in Python\n",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, the correct capitalisation is bAbI6

Suggested change
"# Tutorial: babI6 Training and Preprocessing in Python\n",
"# Tutorial: bAbI6 Training and Preprocessing in Python\n",

"source": [
"Before we delve into the code, we first highlight two new features of the new parser that will be used in this tutorial: the sandwich functor and foliated frames. \n",
"\n",
"In the previous versions of the parser, the semantic functor, while providing quantum implementations for boxes, wires and states, did not specify the quantum implementation of frames. The sandwich functor addresses this issue by introducing a novel construction that breaks down a frame into a sequence of boxes with the frame's contents. Now that we have these different frames, we can decide whether we want every layer in these frames to be assigned their operator or have the same operator for all the layers (different parameters assigned to the layers as opposed to having all the layers having the same parameter). For more detail on this, we recommend reading the paper explaining the theory behind the new parser [here](in_the_making)."
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe break this block up and show the decomposition of a simple example sentence, or link to the other discocirc-basics tutorial.

" List[str]]:\n",
" \"\"\"\n",
" reads the .txt file at `path`\n",
" returns 3 lists of equal length\n",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
" returns 3 lists of equal length\n",
" returns 4 lists of equal length\n",

Comment on lines +175 to +182
" processed_texts_list = []\n",
" for text in texts:\n",
" processed_text = \"\"\n",
" for sentence in text:\n",
" processed_text += sentence + \". \"\n",
" \n",
" processed_texts_list.append(processed_text)\n",
" \n",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
" processed_texts_list = []\n",
" for text in texts:\n",
" processed_text = \"\"\n",
" for sentence in text:\n",
" processed_text += sentence + \". \"\n",
" \n",
" processed_texts_list.append(processed_text)\n",
" \n",
" processed_texts_list = [\n",
" \". \".join(text)\n",
" for text in texts\n",
" ]\n",

simplify

Comment on lines +166 to +168
" (text, question, answer, text_length)\n",
" for text, question, answer, text_length in zip(texts, questions, answers, text_length)\n",
" if len(text) <= TEXT_LENGTH\n",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
" (text, question, answer, text_length)\n",
" for text, question, answer, text_length in zip(texts, questions, answers, text_length)\n",
" if len(text) <= TEXT_LENGTH\n",
" (text, question, answer, text_length)\n",
" for text, question, answer, text_length in zip(texts, questions, answers, text_length)\n",
" if len(text) <= TEXT_LENGTH\n",

Comment on lines 477 to 484
" quest_mid_layer = Id(qubit) if (qid1 == 0 or qid2 == 0) else Discard()\n",
" \n",
" \n",
" for k in range(1, len(text_circuit.cod)):\n",
" if k == qid1 or k == qid2:\n",
" quest_mid_layer = quest_mid_layer @ Id(qubit)\n",
" else:\n",
" quest_mid_layer = quest_mid_layer @ Discard()\n",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, this feels clearer, but up to you.

quest_mid_layer = Id().tensor(*[
    Discard() if k in [qid1, qid2] else Id(qubit)
    for k in range(len(text_circuit.cod))
])

Comment on lines 556 to 566
"# Add the 'measure' field to each item\n",
"for key, value in babi6_datadict.items():\n",
" temp = -1 if value['answer'] == 0 else 1\n",
" value['measure'] = temp * value['text_length']\n",
"\n",
"# Group items by absolute value of measure\n",
"abs_value_groups = defaultdict(list)\n",
"for key, value in babi6_datadict.items():\n",
" abs_value = abs(value['measure'])\n",
" abs_value_groups[abs_value].append((key, value))\n",
"\n",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its probably clearer to avoid reference to the measure key here and keep it in terms of the text length/ans class explicitly, since abs(measure) == text length and measure > 0 == (ans == 'yes').

You can even avoid translating the answers into numbers as per the cell above and ignore that step.

"id": "99b9ac0c",
"metadata": {},
"source": [
"We also modify the yes and no answers and replace their representations by 1s and 0s, i.e. by \"[1, 0]\"s and \"[0, 1]\"s respectively."
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"We also modify the yes and no answers and replace their representations by 1s and 0s, i.e. by \"[1, 0]\"s and \"[0, 1]\"s respectively."
"We also modify the yes and no answers and replace their representations with a one-hot encoding over the possible assertions, i.e. by \"[1, 0]\"s and \"[0, 1]\"s respectively."

"metadata": {},
"source": [
"## 8. Training the Circuits and Tests\n",
"Now that we have the data ready, we proceed with the training as usual, except that, we have to deal with pairs of circuits instead of single circuits, which will be accommodated by overriding the forward() method in the model."
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should maybe also add here that the final output of the model is going to be a vector we can interpret as a probability distribution over the possible answers (in this case [yes, no]). This should give better context to the next cell where you update the answer encodings.

"metadata": {},
"outputs": [],
"source": [
"all_circuits = []\n",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplify

all_circuits = [
    circuit
    for circuit_tuple in training_circuits + val_circuits + test_circuits
    for circuit in circuit_tuple
]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants