pasqal-io / qadence-libs

A collection of libraries to enhance Qadence functionalities.
https://pasqal-io.github.io/qadence-libs/latest/
Apache License 2.0
6 stars 2 forks source link

[Draft] Add a standard way of creating QNN models #18

Closed smitchaudhary closed 4 months ago

smitchaudhary commented 4 months ago

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.

dominikandreasseitz commented 4 months ago

hi @smitchaudhary , i responded in https://github.com/pasqal-io/qadence/issues/396 regarding this, but i think @jpmoutinho is the best person to consult about this

jpmoutinho commented 4 months ago

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

smitchaudhary commented 4 months ago

Thanks @dominikandreasseitz and @jpmoutinho

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

n-toscano commented 4 months ago

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

Roland-djee commented 4 months ago

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

smitchaudhary commented 4 months ago

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

dominikandreasseitz commented 4 months ago

@smitchaudhary should we merge https://github.com/pasqal-io/qadence/pull/385 and this together?

smitchaudhary commented 4 months ago

@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 commented 4 months ago

@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 commented 4 months ago

@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:

smitchaudhary commented 4 months ago

@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 commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

Moved this PR to the main qadence repo here