-
Notifications
You must be signed in to change notification settings - Fork 0
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] Code prototypes #24
base: main
Are you sure you want to change the base?
Conversation
…ion' into em/hotfix/observable_on_expectation
remove import error message for tkinter
remove import error message for tkinter
remove import error message for tkinter
remove import error message for tkinter
…ion' into em/hotfix/observable_on_expectation
Signed-off-by: Doomsk <[email protected]>
Signed-off-by: Doomsk <[email protected]>
add extra functionalities to accommodate proper usage of the package Signed-off-by: Doomsk <[email protected]>
add extra functionalities to accommodate proper usage of the package Signed-off-by: Doomsk <[email protected]>
Signed-off-by: Doomsk <[email protected]>
Signed-off-by: Doomsk <[email protected]>
Signed-off-by: Doomsk <[email protected]>
Signed-off-by: Doomsk <[email protected]>
fix minor mypy inconsistencies Signed-off-by: Doomsk <[email protected]>
Signed-off-by: Doomsk <[email protected]>
Signed-off-by: Doomsk <[email protected]>
Signed-off-by: Doomsk <[email protected]>
Signed-off-by: Doomsk <[email protected]>
Signed-off-by: Doomsk <[email protected]>
Signed-off-by: Doomsk <[email protected]>
Signed-off-by: Doomsk <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Eduardo, thank you for the good work on this PR. I have a few comments to be addressed.
Also, I see that the test suite doesn't have 100% coverage yet, could you have a look into the parts of the code that are not tested yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this import + default type setting be pushed down into the pyqtorch subfolder? If you're using a different backend importing torch is not necessary.
values: dict[str, ArrayType] | None = None, | ||
callback: Optional[Callable] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstring should reflect this change.
values: dict[str, ArrayType] | None = None, | ||
shots: int | None = None, | ||
callback: Optional[Callable] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstring should reflect this change
values: dict[str, ArrayType] | None = None, | ||
observable: Any | None = None, | ||
callback: Optional[Callable] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstring to be updated.
""" | ||
Convert InputType object to Qutip native quantum objects for simulation on QuTiP. | ||
|
||
It is intended to be used on expectation method of the Fresnel1 interface class. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
used on the expectation method
from typing import Any, Protocol, runtime_checkable | ||
|
||
|
||
@runtime_checkable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why should the protocols in this module be runtime_checkable?
if observable is not None: | ||
return platform.expect( | ||
obs_list=parse_native_observables( | ||
num_qubits=len(self.sequence.register.qubit_ids), observable=observable | ||
) | ||
) | ||
raise ValueError("observable cannot be None or empty on 'expectation' method.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reducing the number of indents:
if observable is not None: | |
return platform.expect( | |
obs_list=parse_native_observables( | |
num_qubits=len(self.sequence.register.qubit_ids), observable=observable | |
) | |
) | |
raise ValueError("observable cannot be None or empty on 'expectation' method.") | |
if observable is None: | |
raise ValueError("observable cannot be None or empty on 'expectation' method.") | |
return platform.expect( | |
obs_list=parse_native_observables( | |
num_qubits=len(self.sequence.register.qubit_ids), observable=observable | |
) | |
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be a good idea to group the tests per backend? This way they stay organized when there are many more backends implemented. Like so:
├── tests
│ ├── fresnel1
| | └── test_xxx.py
│ ├── pyqtorch
| | └── test_yyy.py
This way the folder structure also matches more closely with the source structure.
An additional suggestion is to define conftest.py
per backend for fixtures that are backend specific.
Writing all the code prototypes in Qadence 2 that @jpmoutinho is generating on Qadence 1.
Regardless the examples, some points to consider in the TODO list:
[ ] properly handle
dtype
s (João pointed out to include theset_default_dtype(torch.float64)
in the__init__.py
file ofqadence2-core
folder)[ ] fix the issue with the
inputs
on theqadence2_platforms.backends.pyqtorch.embedding.Embedding.__call__
method containing variational parameters (variables) as well[ ] replace dictionaries to
OrderedDict
on the inputs and/or variational parameters