Allow custom range sequence in StronglyEntanglingLayers

[wongwsvincent]

Context:
Fixing a bug to allow the usage of custom range sequence in StronglyEntanglingLayers

Description of the Change:
The ranges parameter can now be defined properly if a specific sequence is defined by the user.
The range value is checked such that it's not zero or divisible by the number of wires.
If the range sequence length is longer than the number of layers , then it takes only the first range values in the sequence.
If the range sequence length is shorter than the number of layers, the range sequence is repeated.
A test is also added to test the custom range sequence feature.

Related GitHub Issues:
Resolves issue #1330

[codecov]

Codecov Report

Merging #1332 (f412d05) into master (79852c9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1332   +/-   ##
=======================================
  Coverage   98.16%   98.16%           
=======================================
  Files         145      145           
  Lines       11093    11099    +6     
=======================================
+ Hits        10889    10895    +6     
  Misses        204      204           

Impacted Files	Coverage Δ
pennylane/templates/layers/strongly_entangling.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79852c9...f412d05. Read the comment docs.

[josh146]

Thanks for this nice contribution @wongwsvincent! Is this PR now ready for review?

[wongwsvincent]

Thanks for this nice contribution @wongwsvincent! Is this PR now ready for review?

Hi @josh146 , I am having trouble not decreasing the coverage of the tests. I included a raise ValueError function to prevent the user from using range values equal to zero or divisible by the number of wires, but that line of code would never be reached by the tests. I tried including a bad parameter test case that would cause the error, but that would fail the core-tests as expected. I have just reverted back to the previous version by removing that bad test case. Let me know if there's something I can try to avoid the decrease in coverage; otherwise, this PR is ready for review.

[mariaschuld]

Thanks so much for this contribution @wongwsvincent, this is great!

I left a few comments, but most are just nitpicking, and I am happy to be convinced otherwise...

About the coverage: you can (and should) test all errors with the context with pytest.raises(ValueError, match="<part_of_the_error_message>"):, in which you can create the op. If the error is not raised, the test will fail. Would that help, or did I misunderstand your problem?

[wongwsvincent]

Thanks so much for reviewing the code, @mariaschuld .
And thanks for the tips of using pytest.raises. It sounds exactly what I am looking for! I will add the error check in to maintain good coverage.

[mariaschuld]

Hey @wongwsvincent .

Thanks for updating the PR - you notice that we are quite strict with code review :)

The only last change would be to move the error tests into a separate test function, then it's ready to be merged from my side! Just ask for a new review when this is done.

Oh, and don't forget to add yourself to the list of contributors in the CHANGELOG (at the bottom of the new version's release notes) if you're not already there!

[wongwsvincent]

I actually appreciate the strict code review a lot. It's my first time contributing and I definitely learned a lot =D

I have moved the check of error raising regarding the range values to a separate test function. And gratefully added my name in the CHANGELOG

Code should be ready for re-review @mariaschuld

[mariaschuld]

Perfect, from my side ready to merge!