[Bug] MultivariateStrategy.SERIES has different behaviors when passed to circuit or directly to QNN

[n-toscano]

Short description

When creating a FM that takes more than one variable as input, we need the two encodings not to commute with each other. If the config is passed directly to QNN.from_configs() then an entangling layer is placed in between, and all works. It's not the case if one passes it to the QuantumCircuit.

What is the expected result?

No response

What is the actual result?

No response

Steps/Code to reproduce

from qadence import *

n_qubits = 4
fm_config = FeatureMapConfig(
    num_features=2,
    inputs = ["x", "y"],
    multivariate_strategy= MultivariateStrategy.SERIES,
)
observable_config = ObservableConfig(detuning=Z)
qnn = QNN.from_configs(
    register=n_qubits,
    fm_config=fm_config,
    obs_config=observable_config
)

vs

fm = create_fm_blocks(register=n_qubits, config=fm_config)
circuit = QuantumCircuit(support=n_qubits, fm)

Tracebacks (optional)

No response

Environment details (optional)

No response

Would you like to work on this issue?

Yes

[mlahariya]

To review.

[mlahariya]

Hey @n-toscano - are you planning to work on this, or possibly we can pick it up?

[n-toscano]

I can pick it up! Can we add it in the current (or next) sprint?

[mlahariya]

I can pick it up! Can we add it in the current (or next) sprint?

Perfect. Sure, we can add it or either current or next sprint. For now, I'll just put it in the next sprint.

[smitchaudhary]

So about this, I think one straightforward way would be to rename what the functions are called so they are a bit more clear?

I did not have in mind that users would want to create a QuantumCircuit and not a QNN directly from the configs, so this process of interleaving an ansatz layers between the fm blocks is a bit opaque at the moment.

If you use the output of the _interleave_ansatz_in_fm to generate the QuantumCircuit then it should be okay but I guess the naming is something we need to rectify.

I would suggest something like "encoding_blocks_list" instead of "create_fm_blocks" to emphasize that these are the encoding blocks and the user might have a different notion of what forms the full fm (also the ansatz layer in between)? And then the _interleave_ansatz_in_fm` should also be renamed to something more clear along the lines of "full_fm" or something.

The reason that we have this weird function called create_fm_blocks which outputs a list of blocks instead of a single block (The other two constructors for Ansatz and Observable give you the respective block directly) is:

1. We don't want a single block yet because you want to interleave the ansatz blocks in there.
2. We want the user to be able to create the blocks with just passing the FeatureMapConfig.

Renaming the functions like I suggest would still be okay with point 1 above, but then in order to get the full FM, the user would also have to provide the AnsatzConfig.

[n-toscano]

Thanks for the contribution, @smitchaudhary.

I must say that when I don't build a custom QNN from scratch, I use QNN.from_configs() rather than passing the config to a QuantumCircuit and then building a QNN.

I see your point though, and the only way through I see would be making QuantumCircuits behave similarly to QNNs regarding configs, but this may be counterintuitive and not so useful for the community. The reason I thought this was a bug was because create_fm_blocks, create_ansatz and observable_from_config are user accessible, but if I understand correctly they should only be called within the QNN constructor to build the underlying blocks.

Side note, could we rename observable_from_config to match the other functions, e.g. create_observable?

[smitchaudhary]

but this may be counterintuitive and not so useful for the community.

Agreed.

The reason I thought this was a bug was because create_fm_blocks, create_ansatz and observable_from_config are user accessible, but if I understand correctly they should only be called within the QNN constructor to build the underlying blocks.

Yes, you are right. And sorry for causing the confusion. Some less than ideal design decisions on my part which are coming to bite us now.

Among other such small reasons, one reason to make them user accessible, compared to all other constructors which start with _ and then marked as not for the user to see, is because I wanted to show the inner workings in a tutorial. :/

Side note, could we rename observable_from_config to match the other functions, e.g. create_observable?

Yes. There are some potentially confusing functions related to observable construction. Very happy to have them renamed appropriately.

I would ping @minutogiovanni since he is reworking observable configs anyways in #659

[minutogiovanni]

Side note, could we rename observable_from_config to match the other functions, e.g. create_observable?

Good point! I can manage this in my PR

[n-toscano]

So the only remaining point still open is (possibly) renaming the functions? I'm not against keeping them as they are (apart from observable_from_config which will be addressed in #659) if they are included in a tutorial that guides the user through configs.