Make tape.trainable_params a list
#1904
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1904 +/- ##
=======================================
Coverage 98.90% 98.90%
=======================================
Files 219 219
Lines 16790 16791 +1
=======================================
+ Hits 16606 16607 +1
Misses 184 184
Continue to review full report at Codecov.
|
| * `QuantumTape.trainable_params` now is a list instead of a set. This | ||
| means that `tape.trainable_params` will return a list unlike before, | ||
| but setting the `trainable_params` with a set works exactly as before. | ||
| [(#1xxx)](https://github.com/PennyLaneAI/pennylane/pull/1xxx) |
There was a problem hiding this comment.
| [(#1xxx)](https://github.com/PennyLaneAI/pennylane/pull/1xxx) | |
| [(#1904)](https://github.com/PennyLaneAI/pennylane/pull/1904) |
we're almost at the #2000 milestone 🤯
There was a problem hiding this comment.
@dwierichs I think you forgot this one 😆 I'll incorporate it into #1807, I'm currently resolving changelog conflicts over there
There was a problem hiding this comment.
Yeah, sorry, I forgot the second occurrence of the PR number in there 😅 Maria already pointed this out to me as well :D
Thanks for taking care of it! :)
| self._par_info.keys() is assumed to be sorted | ||
| As its order is maintained, this assumes that self._par_info | ||
| is created in a sorted manner, as in _update_par_info |
|
|
||
| # get the corresponding operation parameter index | ||
| # (that is, index of the parameter within the operation) | ||
| p_idx = self._par_info[t_idx]["p_idx"] | ||
| p_idx = info["p_idx"] |
Context:
Recently, the
trainable_paramsproperty ofQuantumTapes, which is aset, caused bugs based on the fact that it is not guaranteed to maintain its ordering.In particular for larger tapes these rare instances seem to occur, which likely is the reason why it was not caught before.
As an example, consider the "unordered" set
{3, 1}. As integer hashes are the integers themselves, a "canonical" ordering for python is according to the hash table, so that>>> {3, 1} {1, 3}This makes constructing small tests that fail based on
trainable_paramsbeing a set quite hard.Description of the Change:
QuantumTape.trainable_paramsis made alistinstead, with an intermediate conversion tosetand subsequent sorting within the attribute setter function to exclude duplicates and assure the entries are sorted. Whenever the uniqueness and ordering of the entries is guaranteed, the private attribute_trainable_paramsmay be set directly, as is done in_update_trainable_params.Benefits:
Fixes rare but apparently increasingly important edge case bugs where the operation/parameter ordering was disturbed by the loss of ordering within
trainable_params.Possible Drawbacks:
For very large tapes with many trainable parameters, a lookup will be slower.
Related GitHub Issues: