-
Notifications
You must be signed in to change notification settings - Fork 66
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
Create tools to build operators #706
Open
a-corni
wants to merge
30
commits into
develop
Choose a base branch
from
operators
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
8e95a93
Associate states to bases
a-corni 4f741fb
Add states to conventions
a-corni 5ac64e9
Use states in Hamiltonian
a-corni 31c9e2c
Add Channel.eigenstates, corespondance btw eigenstates and labels
a-corni 19d85b7
Fixing widths in table
a-corni 583030d
Adding operators.py
a-corni 392b626
Fixing typo
a-corni 8d46c70
Revert changes to convetions, make table in Channels docstring
a-corni b198aab
Add r"""
a-corni db505f3
Fix indentation
a-corni 0d7892b
Merge branch 'develop' of https://github.com/pasqal-io/Pulser into st…
a-corni 8390374
Fix table in eigenstates docstring
a-corni a121e8f
Fix typo
a-corni 77942a8
Merge branch 'states' into operators
a-corni 5df5384
Replace build_operator, make tests
a-corni a79099e
Add multiple_bases_states, check for eigenstates
a-corni e649097
Sort imports
a-corni c5188fe
Move test on EIGENSTATES to unit tests
a-corni c8bf660
Change name of multiple_bases_states
a-corni a2da7de
Fix typo
a-corni c71b9d3
Draft: Adding QuditOperator and MultiQuditOperator
a-corni 702721a
Fix import of Collection
a-corni bda1fea
Merge branch 'states' into operators
a-corni 523a182
Merge branch 'develop' of https://github.com/pasqal-io/Pulser into op…
a-corni 1cd3722
Create operator.py in Pulser
a-corni cc81e03
Add check_eigenbasis
a-corni d94d410
Use eigenbasis in build_operator, define QutipOperator, fix typing
a-corni 05d59ae
Delete QuditOperator and MultiQuditOperator
a-corni 170e969
Implement kronecker product
a-corni e342527
Create Hamiltonian class
a-corni File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
# Copyright 2024 Pulser Development Team | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
"""A class to define the Pulser Hamiltonian.""" | ||
from __future__ import annotations | ||
|
||
from pulser.operators.operator import TimeOperator | ||
from pulser import Sequence | ||
from pulser.register.base_register import BaseRegister | ||
from pulser.devices._device_datacls import BaseDevice | ||
from pulser.sampler.samples import SequenceSamples | ||
|
||
|
||
class Hamiltonian(TimeOperator): | ||
|
||
@classmethod | ||
def from_samples( | ||
cls, | ||
sampled_seq: SequenceSamples, | ||
register: BaseRegister, | ||
device: BaseDevice, | ||
sampling_rate: float = 1.0, | ||
) -> Hamiltonian: | ||
pass | ||
|
||
@classmethod | ||
def from_sequence(cls, sequence: Sequence) -> Hamiltonian: | ||
pass |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I could use some guidance here.
I have just figured that with the new operators, we can build the Hamiltonian without Qutip. We could then take out the Hamiltonian class out of pulser-simulation such that it can be used by the other frameworks ?
In pulser-simulation, I would put a "QutipHamiltonian" that would convert the Hamiltonian into a QobjEvo.
However, this might go a bit beyond the scope of this PR and I think it should be left as an issue after this PR is merged
What do you think @HGSilveri @a-quelle ?
Could also mention @lvignoli whom I discussed this possibility with.
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.
What is the need for this? I'm not familiar with pulser simulation, but I suspect it's a bit specific to there. I'm currently implementing something very similar to the
Backend
ABC, and I have a run method likeso there is no real reason to explicitly store a time series anywhere. If the user wants to measure an operator, the can create it from the
Operator
ABC that you created below.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.
The idea with this Hamiltonian(TimeOperator) is that it would avoid you making _extract_omega_delta, get_qubit_positions, and make_H, you could just do Hamiltonian.from_sequence(sequence, sampling_times), that would make a list of mpo_t (as Operator) and time steps, and then you could convert these Operators into mpo_t (as matrix product states, with the converter you have defined).
That could enable us to share the same definition of the Pulser Hamiltonian ? I think this could be useful for other emulators as well.
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 code efficiency reasons, I would avoid that code path even if it were available. Having an abstract representation for operators and states is unavoidable if you want a backend independent interface, but within the backend, everything goes. Constructing the Hamiltonians by going through the abstract format that we're now defining would be at least an order of magnitude slower than the bit fiddling I'm doing in
make_H
, as well as creating problems with differentiability due tosvd
operations in the generalMPO
addition algorithm that I took some pains to avoid inmake_H
.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.
I appreciate that you're trying to be helpful, but that is not your job. Rather it's my job to be helpful by removing the burden what logic is needed to actually efficiently simulate a pulse in a given backend from the pulser team.
For historical reasons, it doesn't quite work that way yet, but there is no time like the present to try and improve things.
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.
I also think this would severely complicate things for marginal benefit. If the Hamiltonian was a single operator I could see it, but the time-dependence seems like a deal breaker to me
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.
Thanks for your feedback @a-quelle and @HGSilveri, let's forget about this feature then.