Skip to content
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] Add a standard way of creating QNN models #18

Closed
wants to merge 3 commits into from

Conversation

smitchaudhary
Copy link
Collaborator

@smitchaudhary smitchaudhary commented Apr 9, 2024

Meant to fix pasqal-io/qadence#396

It is not meant as a replacement of the current workflow but a more of a standardised alternative.

  • Main motivation is to have a standard manner of defining QNNs.
  • Good for quick prototyping and benchmarking different parts (Rydberg Hamiltonian based ansatz vs a fully digital ansatz etc)
  • Much easier to store this
  • Reduces a bunch of boilerplate code that you need to write.

@smitchaudhary smitchaudhary self-assigned this Apr 9, 2024
@smitchaudhary smitchaudhary changed the title [Draft] Add a standard wayof creating QNN models [Draft] Add a standard way of creating QNN models Apr 9, 2024
@dominikandreasseitz
Copy link
Collaborator

hi @smitchaudhary , i responded in pasqal-io/qadence#396 regarding this, but i think @jpmoutinho is the best person to consult about this

@jpmoutinho
Copy link
Collaborator

jpmoutinho commented Apr 11, 2024

Thanks @dominikandreasseitz, yes I am aware of these changes and will review it.

@smitchaudhary
Copy link
Collaborator Author

Thanks @dominikandreasseitz and @jpmoutinho

Still a draft. Once I incorporate more changes, will request a proper review. Open for feedback already though.

@n-toscano
Copy link
Collaborator

Watchout @smitchaudhary, there's still a reference to the eic_aero package in the quantum_models file

@RolandMacDoland
Copy link
Collaborator

Hey @smitchaudhary will you have time to work on this ?

@smitchaudhary
Copy link
Collaborator Author

@Roland-djee Yes I will pick it up soon. Gathering some more feedback from some users and incorporating that.



@dataclass
class ObservableConfig:
Copy link
Collaborator

Choose a reason for hiding this comment

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

hey @smitchaudhary , lets add the logic for output transformation here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah! It can already do a scaling operation for each qubit. Will expand that to include shifting too. Also I need to make the names a bit more clear.

@dominikandreasseitz
Copy link
Collaborator

@smitchaudhary should we merge pasqal-io/qadence#385 and this together?

@smitchaudhary
Copy link
Collaborator Author

@dominikandreasseitz I assume the transformed module needs to be refactored for qadence at large but does this new config workflow also go directly into qadence, in contrast to just qadence-libs for now?

@dominikandreasseitz
Copy link
Collaborator

@dominikandreasseitz I assume the transformed module needs to be refactored for qadence at large but does this new config workflow also go directly into qadence, in contrast to just qadence-libs for now?

imo, i would have all of this in qadence for now and then move the full ml_tools module once its ready @Roland-djee @jpmoutinho @Doomsk @smitchaudhary

@jpmoutinho
Copy link
Collaborator

@dominikandreasseitz I assume the transformed module needs to be refactored for qadence at large but does this new config workflow also go directly into qadence, in contrast to just qadence-libs for now?

imo, i would have all of this in qadence for now and then move the full ml_tools module once its ready @Roland-djee @jpmoutinho @Doomsk @smitchaudhary

I would first try to converge on the goal for the QNN interface before deciding how to handle the code here and the changes being made in your MR @dominikandreasseitz.

Some thoughts:

  • In the current state, the code here is a convenience layer on top of QNN which culminates in build_qnn function. Is that desired? Another option is to have the configs be part of the input of the QNN class itself.
  • In this MR the scaling parameters affect how the feature map constructor is built, while in the QNN MR the scaling affects the parameters of the QNN itself. These are functionalities with an overlapping goal but different implementation. What is the plan on combining those?
  • Same thing for the output scaling and observable config.

@smitchaudhary
Copy link
Collaborator Author

smitchaudhary commented Apr 22, 2024

@jpmoutinho

In the current state, the code here is a convenience layer on top of QNN which culminates in build_qnn function. Is that desired? Another option is to have the configs be part of the input of the QNN class itself.

This was developed independent of the QNN/TransformedModule logic (and is just a draft at that), so if the plan is to open up QNN and change the definition/signature altogether, then we could do that. However, the current QuantumCircuit class is a lot more general than what is created inside build_qnn_model. For starters, in general, a circuit is not required to have a feature map, or ansatz. It can have any block. All this to say that if this is the route taken, then the current logic with the configs here will need to be changed accordingly.

These are functionalities with an overlapping goal but different implementation. What is the plan on combining those?

I think @dominikandreasseitz 's impetus behind merging the two is exactly this. Do away with the wrapper TransformedModule.

@jpmoutinho
Copy link
Collaborator

In the current state, the code here is a convenience layer on top of QNN which culminates in build_qnn function. Is that desired? Another option is to have the configs be part of the input of the QNN class itself.

This was developed independent of the QNN/TransformedModule logic (and is just a draft at that), so if the plan is to open up QNN and change the definition/signature altogether, then we could do that. However, the current QuantumCircuit class is a lot more general than what is created inside build_qnn_model. For starters, in general, a circuit is not required to have a feature map, or ansatz. It can have any block. All this to say that if this is the route taken, then the current logic with the configs here will need to be changed accordingly.

Yes indeed! I just meant that it is an opportunity to think about the best option and work towards that. We already have the QuantumModel for general circuits, and we could allow the QNN to be initialized with your workflow directly, letting your code take care of the circuit initialization. It could still accept custom circuits of course. I understand it is currently designed as a layer on top, and that is also fine by me if you prefer that as a user.

@dominikandreasseitz
Copy link
Collaborator

@awennersteen @Roland-djee , @smitchaudhary @jpmoutinho and I propose to combine https://github.com/pasqal-io/qadence/pull/385/files and this MR together, meaning:

  1. Removal of the TranformedModule
  2. Achieving scaling/shifting of inputs and outputs of QNNs through featuremap and observable configs

However some libraries which use qadence rely on the TransfomedModule for classical NNs, we think its best to fully separate the quantum and classical parts which would be achieved by doing the above. wdyt?

@smitchaudhary
Copy link
Collaborator Author

Moved this PR to the main qadence repo here

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.

[Feature] Add helper functions to readily create QNNs
5 participants