Add tutorial for hybrid use-case. #22
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MatteoRobbiati
requested changes
Jun 20, 2025
Contributor
MatteoRobbiati
left a comment
MatteoRobbiati
left a comment
There was a problem hiding this comment.
Many, many thanks for this contribution!
I left a list of comments (some of them are really minor suggestions) below.
Contributor
There was a problem hiding this comment.
Some comments on the notebook:
- I would rename the file into something like
lie_equivariant_quantum_graph_nn.ipynb. Then, as you do in the text, we can mention it is a very good example for hybrid architectures :) - QiBO --> Qibo in the first cell;
- Add some docstrings to function defined in code cell [2];
- I think the data are missing. Probably you just forgot to upload them into
../data/.; - In the text of section 2. we define the graph with normal text font, while in the previous section
\mathcalis used. I would choose one of the two; - I would standardize the figures' captions with the way we do it in Qiboedu. Namely, Figure n: description. Nothing particularly relevant, it is just to have a uniform style;
- Add docstrings to functions in cells [5-7];
- I would move the Auxiliary functions to a proper file within the
qiboedu/src/qiboedu/scriptsfolder, to lighten the notebook :) Of course, you also have to tell me if you prefer to keep these here. But I believe we could just mention that all the scripts are available in the aforementioned path; - In cell [8] and [9] there are some imports already done in the cell [1]. Please, clean the imports and move all the necessary to the first cell;
- I was thinking if to move the model in some script as well, but it can be actually interesting to keep it here. I don't have any particular strong opinion on this. If we keep it, please add docstrings to all the methods of the model;
- I would add some comment to guide the reading of cells [10-15];
- Same as before, in cell [16] there are redundant imports;
- I would consider lighten the output during the training (e.g. removing
tqdm), and, if it is not necessary to understand the tutorial, I would also move part of the training functions to scripts; - cell [17] is probably still script-oriented. For example the
if __name__ == "main", and the usage of global variable definition. I think this can be a bit confusing, so I would move the part to a script or make it a bit more notebook-friendly; - I find curious how validation loss and accuracy are actually better than the training one. Do you have any idea on why this is happening?
- I would standardize the plots to the style proposed in
qiboedu/src/qiboedu/scripts/plotscriptsin terms of color, inches, etc. Please let me know if any help is needed :)
Contributor
Author
There was a problem hiding this comment.
Hi! I'm working on the points above. Specifically referring to [8, 14], in order to upload the scripts in qiboedu/src/qiboedu/scripts, I need push permission, please.
Add functions to plot accuracies and losses of models.
These two files contain training scripts and some utilitary functions for lie_eqgnn, like computing Minkowski invariant features, GradualWarmupScheduler, etc.
MatteoRobbiati
requested changes
Jul 2, 2025
Comment on lines
+52
to
+55
| "from qiboedu.scripts.lie_eqgnn_utils import *\n", | ||
| "from qiboedu.scripts.plotscripts import plot_loss, plot_accuracies\n", | ||
| "from qiboedu.scripts.lie_eqgnn_train import *\n", | ||
| "\n", |
Contributor
There was a problem hiding this comment.
I am now a big fan of from package import *, for readability and also because it is not often necessary to import all. I would prefer importing the single functions/classes individually :)
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": [ | ||
| "# to begin loading the jets, we will need some auxiliary functions to handle the graphical format.\n", |
Contributor
There was a problem hiding this comment.
Suggested change
| "# to begin loading the jets, we will need some auxiliary functions to handle the graphical format.\n", | |
| "# To begin loading the jets, we will need some auxiliary functions to handle the graphical format.\n", |
| "def get_adj_matrix(n_nodes, batch_size, edge_mask):\n", | ||
| "\n", | ||
| " \"\"\"\n", | ||
| " Constructs a batched edge list (adjacency list) from a binary edge mask\n", |
Contributor
There was a problem hiding this comment.
Suggested change
| " Constructs a batched edge list (adjacency list) from a binary edge mask\n", | |
| " Construct a batched edge list (adjacency list) from a binary edge mask\n", |
This is extremely pedantic .-. but, according to the PEP 257 Docstring Conventions:
The docstring is a phrase ending in a period. It prescribes the function or method’s effect as a command (“Do this”, “Return that”), not as a description; e.g. don’t write “Returns the pathname …”.
| "def load_jets(p4s, nodes, labels, atom_mask, edge_mask, edges, batch_size, train_ratio, val_ratio, test_ratio):\n", | ||
| "\n", | ||
| " \"\"\"\n", | ||
| " Splits the full jet dataset into training, validation, and test sets,\n", |
Contributor
There was a problem hiding this comment.
Suggested change
| " Splits the full jet dataset into training, validation, and test sets,\n", | |
| " Split the full jet dataset into training, validation, and test sets,\n", |
As before :)
| "val_ratio = 0.1\n", | ||
| "test_ratio = 0.1\n", | ||
| "\n", | ||
| "dataloaders = load_jets(p4s, nodes, labels, atom_mask, edge_mask, edges, 16, train_ratio, val_ratio, test_ratio)" |
Contributor
There was a problem hiding this comment.
I think we can just expose the call to this load_jets function, and move the loading utils into dedicated script file, to lighten the readability. But I am open to discuss about it.
| " def m_model(self, hi, hj, norms, dots):\n", | ||
| "\n", | ||
| " \"\"\"\n", | ||
| " Computes Lorentz-invariant edge messages for the graph neural network.\n", |
Contributor
There was a problem hiding this comment.
Suggested change
| " Computes Lorentz-invariant edge messages for the graph neural network.\n", | |
| " Compute Lorentz-invariant edge messages for the graph neural network.\n", |
As before.
| "\n", | ||
| " def h_model(self, h, edges, m, node_attr):\n", | ||
| " \"\"\"\n", | ||
| " Computes Lorentz-invariant scalar latent representation.\n", |
Contributor
There was a problem hiding this comment.
Suggested change
| " Computes Lorentz-invariant scalar latent representation.\n", | |
| " Compute Lorentz-invariant scalar latent representation.\n", |
As before.
Comment on lines
+1219
to
+1220
| "device = 'cuda'\n", | ||
| "dtype = torch.float64\n", |
Contributor
There was a problem hiding this comment.
Suggested change
| "device = 'cuda'\n", | |
| "dtype = torch.float64\n", |
I think this is not needed, since already defined above.
Comment on lines
+1300
to
+1302
| "import os\n", | ||
| "import json\n", | ||
| "import matplotlib.pyplot as plt\n", |
Contributor
There was a problem hiding this comment.
- Remove matplotlib (since already imported above). Also, is it necessary to import it here, after the plotting functions have been moved to the scripts?
- Move the other two imports in the first cell (with the other imports).
Comment on lines
+35
to
+55
| "import numpy as np\n", | ||
| "from math import sqrt\n", | ||
| "import matplotlib.pyplot as plt\n", | ||
| "\n", | ||
| "import torch\n", | ||
| "from torch import nn\n", | ||
| "from torch.utils.data import TensorDataset, DataLoader, random_split\n", | ||
| "from scipy.sparse import coo_matrix\n", | ||
| "\n", | ||
| "from qibo.symbols import Z, I\n", | ||
| "from qibo.hamiltonians import SymbolicHamiltonian\n", | ||
| "from qiboml.models.encoding import PhaseEncoding\n", | ||
| "from qibo import Circuit, gates, hamiltonians\n", | ||
| "from qiboml.models.decoding import QuantumDecoding\n", | ||
| "from qiboml.interfaces.pytorch import QuantumModel\n", | ||
| "from qibo import set_backend\n", | ||
| "\n", | ||
| "from qiboedu.scripts.lie_eqgnn_utils import *\n", | ||
| "from qiboedu.scripts.plotscripts import plot_loss, plot_accuracies\n", | ||
| "from qiboedu.scripts.lie_eqgnn_train import *\n", | ||
| "\n", |
Contributor
There was a problem hiding this comment.
Reorganize the import package by package and follow the PEP 8 guidelines:
- Standard library imports.
- Related third party imports.
- Local application/library specific imports.
(It is already done more or less, but saying this for the moment you will move here os and json - See comment below)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I added one notebook called 3_hybrid_model.ipynb to illustrate a hybrid quark vs gluon classification using QiBO. Besides the notebook, I added two images to Quantum_machine_learning/figures folder.