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

Completed Parameter Implementation #68

Conversation

pafloxy
Copy link

@pafloxy pafloxy commented Jul 6, 2023

Before submitting, please check the following:

  • Make sure you have tests for the new code and that test passes (run pytest)
  • format added code and tests by black -l 120 <filename>
  • If applicable, add a line to the [unreleased] part of CHANGELOG.md, following keep-a-changelog.

Then, please fill in below:

Context (if applicable):

Description of the change:
Introduced Parameter class for providing symbolic placeholder for parameterised arguments in circuit (rotation-angles) / patterns (measurement-angles).

It is allowed to make numerical operation on the instances of the Parameter class which upadates the symbolic expression inplace, however interaction between multiple instances of the class are not allowed .

Numerical value can be assigned using Parameter.bind() method

th = Parameter('th');
np.sin(th/3)
------------------
>Out: expr = sin(th[/3])
th.bind({th:0.3})
-------------------
>Out: expr = sin(th[/3]), assignment = [ th : 0.3 ]
th.value
------------------
>Out: 0.09983341664682814

Related issue:
#45

also see that checks (github actions) pass.
If lint check keeps failing, try installing black==22.8.0 as behavior seems to vary across versions.

shinich1
shinich1 previously approved these changes Jul 6, 2023
@mgarnier59
Copy link
Contributor

Do we validate the behaviour modification of output_nodesin pattern.pyaround line 100? I don't yet understand how it works if the output nodes are specified by the user but if it is handled by the pattern, then I fear this modification will break the code since output_nodes is built iteratively.

@shinich1
Copy link
Contributor

shinich1 commented Jul 7, 2023

Do we validate the behaviour modification of output_nodesin pattern.pyaround line 100? I don't yet understand how it works if the output nodes are specified by the user but if it is handled by the pattern, then I fear this modification will break the code since output_nodes is built iteratively.

Thanks for pointing this out. I agreed to this change in the discussion on the issues page, but I suppose you are right, breaking change like this should be avoided. More so because we don’t need this change for the parametrised patterns to work.

@mgarnier59
Copy link
Contributor

Do we validate the behaviour modification of output_nodesin pattern.pyaround line 100? I don't yet understand how it works if the output nodes are specified by the user but if it is handled by the pattern, then I fear this modification will break the code since output_nodes is built iteratively.

Thanks for pointing this out. I agreed to this change in the discussion on the issues page, but I suppose you are right, breaking change like this should be avoided. More so because we don’t need this change for the parametrised patterns to work.

Maybe the error pointed out by @pafloxy could be resolved by calling the add method directly in __init__.

@shinich1 shinich1 mentioned this pull request Oct 25, 2023
@shinich1 shinich1 dismissed their stale review November 2, 2023 11:56

this may need to be reviewed again with new version of code

@shinich1 shinich1 deleted the branch TeamGraphix:45-parameterized-measurement-patterns May 8, 2024 14:28
@shinich1 shinich1 closed this May 8, 2024
@shinich1
Copy link
Contributor

shinich1 commented May 8, 2024

@pafloxy I accidentally deleted the target branch of this PR, sorry!
what should we do with this feature? I think many of the changes I asked back when we were working on this are now likely unnecessary (sorry..) and your implementation may fit very nicely in the current version of graphix? if you feel like it, please open a PR to merge into master.

@pafloxy
Copy link
Author

pafloxy commented May 8, 2024

Hello @shinich1 ,
Its alright, I think it would be more convenient to rewrite it with the current version of graphix which would also be more convenient. Also we were reconsidering how we use the parameterised class for yourselves among oursleves so maybe this time we will write it in a different fashion, the Parameter class on overall might not be changed but the way it interacts with rest of graphix should be require some re-thinking based on what we decide :)

Comment on lines +89 to +95
if overwrite:
self._value = float(val)
self._assignment = value
else:
if not self.assigned:
self._value = float(val)
self._assignment = value
Copy link
Contributor

@benjvmin93 benjvmin93 May 28, 2024

Choose a reason for hiding this comment

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

In my sense, this part is redundant and should be replaced by something more concise. In any cases, you are setting self._value and self._assignment. For me the conditions should only be if not self.assigned: ... else: raise ValueError("...")

else:
raise ValueError("Symbols are already assigned, set overwrite = True to overwrite value")

if len(val.free_symbols) != 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be replaced by an else statement.

Comment on lines +193 to +210
if inplace:
cmd_indx = -1
for cmd in self.seq:
cmd_indx += 1
if cmd[0] == "M" and isinstance(cmd[3], Parameter):
cmd[3] = cmd[3].bind(parameter_assignment, allow_unknown_parameters=True).value
self.seq[cmd_indx] = cmd

self.inspect_parameterized_commands()

else:
new_pattern = deepcopy(self)
cmd_indx = -1
for cmd in new_pattern.seq:
cmd_indx += 1
if cmd[0] == "M" and isinstance(cmd[3], Parameter):
cmd[3] = cmd[3].bind(parameter_assignment, allow_unknown_parameters=True).value
new_pattern.seq[cmd_indx] = cmd
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, there is some code duplication which could be avoided.

Comment on lines +96 to +119
if inplace:

for gate_indx in parameterized_gate_indx:

gate_instr = self.instruction[gate_indx]
if isinstance(gate_instr[-1], (Parameter)):
gate_instr[-1] = gate_instr[-1].bind(parameter_assignment, allow_unknown_parameters=True).value
print(gate_instr) ##checkflag
self.instruction[gate_indx] = gate_instr

self.inspect_parameterized_gates()

else:

new_pattern = deepcopy(self)

for gate_indx in parameterized_gate_indx:

gate_instr = new_pattern.instruction[gate_indx]
if isinstance(gate_instr[-1], (Parameter)):
gate_instr[-1] = gate_instr[-1].bind(parameter_assignment, allow_unknown_parameters=True).value
new_pattern.instruction[gate_indx] = gate_instr

new_pattern.inspect_parameterized_gates()
Copy link
Contributor

@benjvmin93 benjvmin93 May 28, 2024

Choose a reason for hiding this comment

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

Code duplication that could be avoided by just assigning self.instruction[gate_indx] or new_pattern.instruction[gate_indx] to gate_instr depending on the inplace condition.

if self.num_parameterized_gates == 0:
raise ValueError(" no paramaters to assign ")

elif self.num_parameterized_gates > 0:
Copy link
Contributor

@benjvmin93 benjvmin93 May 28, 2024

Choose a reason for hiding this comment

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

No need to make this part as a condition statement since we know the num_parameterized_gates attribute is setted to 0 and we already test it in the if statement above and that it leads to an error raising if so.

"""
assert qubit in np.arange(self.width)
angle = 1.0 * angle
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand here and in the other rotation instructions why it is necessary to multiply the angle by 1.0. Wouldn't it be interpreted as a parameter object instead of a float anyway? Could you please explain a little more on these lines?

thierry-martinez added a commit to thierry-martinez/graphix that referenced this pull request May 30, 2024
This commit is yet another tentative to implement parameterized
measurement patterns, to fulfill issue TeamGraphix#45 (previous tentative: TeamGraphix#68).

This commit adds two methods to the class `Pattern`:

- `is_parameterized()` returns True if there is at least one
measurement angle that is not just an instance of numbers.Number:
indeed, a parameterized pattern is a pattern where at least one
measurement angle is an expression that is not a number, typically an
instance of `sympy.Expr` (but we don't force to choose sympy here).

- `subs(variable, substitute)` returns a copy of the pattern where
occurrences of the given variable in measurement angles are
substituted by the given value.  Substitution is performed by calling
the method `subs` on measurement angles, if the method exists, which
is the case in particular for `sympy.Expr`. If the substitution
returns a number, this number is coerced to `float`, to get numbers
that implement the full number protocol (in particular, sympy numbers
don't implement `cos`).
thierry-martinez added a commit to thierry-martinez/graphix that referenced this pull request May 30, 2024
This commit is yet another tentative to implement parameterized
measurement patterns, to fulfill issue TeamGraphix#45 (previous tentative: TeamGraphix#68).

This commit adds two methods to the class `Pattern`:

- `is_parameterized()` returns True if there is at least one
measurement angle that is not just an instance of numbers.Number:
indeed, a parameterized pattern is a pattern where at least one
measurement angle is an expression that is not a number, typically an
instance of `sympy.Expr` (but we don't force to choose sympy here).

- `subs(variable, substitute)` returns a copy of the pattern where
occurrences of the given variable in measurement angles are
substituted by the given value.  Substitution is performed by calling
the method `subs` on measurement angles, if the method exists, which
is the case in particular for `sympy.Expr`. If the substitution
returns a number, this number is coerced to `float`, to get numbers
that implement the full number protocol (in particular, sympy numbers
don't implement `cos`).
thierry-martinez added a commit to thierry-martinez/graphix that referenced this pull request May 30, 2024
This commit is yet another tentative to implement parameterized
measurement patterns, to fulfill issue TeamGraphix#45 (previous tentative: TeamGraphix#68).

This commit adds two methods to the class `Pattern`:

- `is_parameterized()` returns True if there is at least one
measurement angle that is not just an instance of numbers.Number:
indeed, a parameterized pattern is a pattern where at least one
measurement angle is an expression that is not a number, typically an
instance of `sympy.Expr` (but we don't force to choose sympy here).

- `subs(variable, substitute)` returns a copy of the pattern where
occurrences of the given variable in measurement angles are
substituted by the given value.  Substitution is performed by calling
the method `subs` on measurement angles, if the method exists, which
is the case in particular for `sympy.Expr`. If the substitution
returns a number, this number is coerced to `float`, to get numbers
that implement the full number protocol (in particular, sympy numbers
don't implement `cos`).
thierry-martinez added a commit to thierry-martinez/graphix that referenced this pull request May 31, 2024
`parameter.py` is inspired by TeamGraphix#68 but is split in Parameter and
ParameterExpression.
thierry-martinez added a commit to thierry-martinez/graphix that referenced this pull request Jul 10, 2024
This commit is yet another tentative to implement parameterized
measurement patterns, to fulfill issue TeamGraphix#45 (previous tentative: TeamGraphix#68).

This commit adds two methods to the class `Pattern`:

- `is_parameterized()` returns True if there is at least one
measurement angle that is not just an instance of numbers.Number:
indeed, a parameterized pattern is a pattern where at least one
measurement angle is an expression that is not a number, typically an
instance of `sympy.Expr` (but we don't force to choose sympy here).

- `subs(variable, substitute)` returns a copy of the pattern where
occurrences of the given variable in measurement angles are
substituted by the given value.  Substitution is performed by calling
the method `subs` on measurement angles, if the method exists, which
is the case in particular for `sympy.Expr`. If the substitution
returns a number, this number is coerced to `float`, to get numbers
that implement the full number protocol (in particular, sympy numbers
don't implement `cos`).
thierry-martinez added a commit to thierry-martinez/graphix that referenced this pull request Jul 10, 2024
`parameter.py` is inspired by TeamGraphix#68 but is split in Parameter and
ParameterExpression.
thierry-martinez added a commit to thierry-martinez/graphix that referenced this pull request Jul 25, 2024
This commit is yet another tentative to implement parameterized
measurement patterns, to fulfill issue TeamGraphix#45 (previous tentative: TeamGraphix#68).

This commit adds two methods to the class `Pattern`:

- `is_parameterized()` returns True if there is at least one
measurement angle that is not just an instance of numbers.Number:
indeed, a parameterized pattern is a pattern where at least one
measurement angle is an expression that is not a number, typically an
instance of `sympy.Expr` (but we don't force to choose sympy here).

- `subs(variable, substitute)` returns a copy of the pattern where
occurrences of the given variable in measurement angles are
substituted by the given value.  Substitution is performed by calling
the method `subs` on measurement angles, if the method exists, which
is the case in particular for `sympy.Expr`. If the substitution
returns a number, this number is coerced to `float`, to get numbers
that implement the full number protocol (in particular, sympy numbers
don't implement `cos`).
thierry-martinez added a commit to thierry-martinez/graphix that referenced this pull request Jul 25, 2024
`parameter.py` is inspired by TeamGraphix#68 but is split in Parameter and
ParameterExpression.
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.

4 participants