Skip to content

Qcnn devel - #221

Open
ihpcganck wants to merge 4 commits into
mainfrom
qcnn-devel
Open

Qcnn devel#221
ihpcganck wants to merge 4 commits into
mainfrom
qcnn-devel

Conversation

@ihpcganck

Copy link
Copy Markdown

qcnn.py was copied from qibo/models but it should be put under qiboml/models. Now it supports 3 variants of QCNN, i.e., QCNNCOMPLEX, QCNNREAL, QCNNRY.

@ihpcganck
ihpcganck requested review from a team June 30, 2026 09:29

@shangtai shangtai left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reminder to include pytests and update the documentations as well.

Comment thread src/qiboml/models/qcnn.py Outdated
Comment thread src/qiboml/models/qcnn.py Outdated
self.nlayers = nlayers


twoqubitansatz = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

twoqubitansatz is an input to the init in line 70 right? If we set it to None here, it would defeat the purpose of taking in the input right?

Comment thread src/qiboml/models/qcnn.py Outdated
Comment thread src/qiboml/models/qcnn.py Outdated
Comment thread src/qiboml/models/qcnn.py Outdated
Comment thread src/qiboml/models/qcnn.py

Args:
bits: the two qubits to apply the unitaries to
symbols: length 15 array containing the parameters

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

bits is assumed to be of length 15, would be good to use assertion to catch if a user input bits of different lengths.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

same as above, no check of length.

Comment thread src/qiboml/models/qcnn.py

Args:
bit: the qubit to apply the one-qubit unitaries to
symbols: length 3 array containing the parameters

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

use assertion to validate that symbols have length 3.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Assertions not done yet, is this by choice?

@scarrazza

Copy link
Copy Markdown
Member

@shangtai could you please have a look at these changes. @renatomello @MatteoRobbiati could you please check if this implementation looks consistent with QiboML?

@shangtai shangtai left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Are we including any pytest for Qcnn?

Comment thread src/qiboml/models/qcnn.py

Args:
bit: the qubit to apply the one-qubit unitaries to
symbols: length 3 array containing the parameters

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Assertions not done yet, is this by choice?

Comment thread src/qiboml/models/qcnn.py

Args:
bits: the two qubits to apply the unitaries to
symbols: length 15 array containing the parameters

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

same as above, no check of length.

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.

3 participants