Closed maliasadi closed 2 weeks ago
Here are some key observations to aid the review process:
β±οΈ Estimated effort to review: 3 π΅π΅π΅βͺβͺ |
π Score: 85 |
π§ͺ No relevant tests |
π No security concerns identified |
β‘ Recommended focus areas for review Possible Bug The `grad` function in `hermite_renormalized` now returns an additional `None` value. This change might affect the behavior of the function and its callers. Code Smell Multiple import statements have been reordered and grouped. This change might affect the readability and organization of the code. |
Explore these optional code suggestions:
Category | Suggestion | Score |
Best practice |
Specify exact versions for dependencies to improve reproducibility___ **Consider specifying exact versions for dependencies to ensure reproducibility,especially for critical packages like TensorFlow.** [pyproject.toml [52-53]](https://github.com/XanaduAI/MrMustard/pull/506/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R52-R53) ```diff tensorflow = {version = "2.17.0" } -tensorflow-intel = { version = ">2.15.0", platform = "win32" } +tensorflow-intel = { version = "2.17.0", platform = "win32" } ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7Why: Specifying exact versions for dependencies can enhance reproducibility by ensuring that the same versions are used across different environments. This is particularly important for critical packages like TensorFlow. | 7 |
Enhancement |
Enhance user guidance by providing an example for the new check option___ **Consider adding a help message for the newcheck option in the format target to improve user guidance.** [Makefile [21]](https://github.com/XanaduAI/MrMustard/pull/506/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R21-R21) ```diff @echo " format [check=1] to run isort and black formatting; use with 'check=1' to check instead of modify" +@echo " Example: make format check=1" ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 5Why: Adding an example for the new `check` option in the help message can improve user guidance and make it easier for users to understand how to use the new feature. This is a minor enhancement but can be helpful for clarity. | 5 |
π‘ Need additional feedback ? start a PR chat
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 89.41%. Comparing base (
5fe4a5f
) to head (60d106c
). Report is 1 commits behind head on develop.
User description
Before submitting
Please complete the following checklist when submitting a PR:
[x] All new features must include a unit test. If you've fixed a bug or added code that should be tested, add a test to the test directory!
[x] All new functions and code must be clearly commented and documented. If you do make documentation changes, make sure that the docs build and render correctly by running
make docs
.[X] Ensure that the test suite passes, by running
make test
.[x] Ensure that code and tests are properly formatted, by running
make format
or `black -l 100[ ] Add a new entry to the
.github/CHANGELOG.md
file, summarizing the change, and including a link back to the PR.[x] The Mr Mustard source code conforms to PEP8 standards except line length. We check all of our code against Pylint. To lint modified files, simply
pip install pylint
, and then runpylint mrmustard/path/to/file.py
.When all the above are checked, delete everything above the dashed line and fill in the pull request template.
Context: This PR fixes the following MM issue with TensorFlow >= 2.15:
This issue arises in custom operations where a gradient function is manually defined, and the expected number of gradients doesn't match the number returned by the function. To fix this issue, we updated all
custom_gradient
use cases in MM to return correct number of gradients. :heavy_check_mark:Python formatting could also be improved with supporting different python version targets in
black
(to avoid formatting inconsistencies when using different py versions) and sorting imports usingisort
.PR Type
enhancement, tests
Description
pyproject.toml
to ensure compatibility with the latest features and improvements.Makefile
to includeisort
for sorting imports and added conditional checks for formatting.backend_tensorflow.py
to return an additionalNone
value, likely for improved error handling or compatibility.isort
as a dependency and configured it alongsideblack
for code formatting.[sc-75755]
Changes walkthrough π
backend_tensorflow.py
Update gradient function to return extra None value
mrmustard/math/backend_tensorflow.py - Modified the `grad` function to return an additional `None` value.
pyproject.toml
Update TensorFlow version and add isort configuration
pyproject.toml
isort
as a dependency.isort
andblack
settings.Makefile
Enhance Makefile with isort and conditional formatting
Makefile
isort
to the format command.conftest.py
Add hypothesis settings import
tests/conftest.py - Added import for `hypothesis.settings`.
test_circuit.py
Reformat import statements for consistency
tests/test_lab/test_circuit.py - Reformatted import statements for consistency.