[Frontend] Make tests toml-schema independent #712

grwlf · 2024-05-03T12:22:37Z

Context: Transition to the quantum device config schema 2

Description of the Change: Solve a regarding toml schema 2 udpate in tests by switching our test custom devices from toml text manipulations to the device capability manipulations

Benefits:

Tests no longer require toml text manipulations.
Tests now contain simple examples of custom devices.
toml-specific code is now locates in catalyst.utils.toml.

Possible Drawbacks:

Related GitHub Issues:
PennyLaneAI/pennylane-lightning#642

dime10 · 2024-05-03T14:39:33Z

Good point, we could just make it compatible with both. I was just going to move to version 2 entirely once the PR is merged. I'm happy with this as well though.

Note there are several other tests that need this update though :)

grwlf · 2024-05-03T19:05:52Z

Note there are several other tests that need this update though :)

@dime10 , are there? I think I changed the device which is used by several tests, and can't see other places.

dime10 · 2024-05-03T19:20:44Z

Note there are several other tests that need this update though :)

@dime10 , are there? I think I changed the device which is used by several tests, and can't see other places.

Have a look at the test_decomposition and test_quantum_control lit tests

codecov · 2024-05-06T12:15:03Z

Codecov Report

Attention: Patch coverage is 97.01493% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 98.07%. Comparing base (47b3482) to head (eadeb91).

Files	Patch %	Lines
frontend/catalyst/device/qjit_device.py	95.12%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #712   +/-   ##
=======================================
  Coverage   98.07%   98.07%           
=======================================
  Files          70       70           
  Lines        9599     9601    +2     
  Branches      754      756    +2     
=======================================
+ Hits         9414     9416    +2     
  Misses        151      151           
  Partials       34       34

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

grwlf · 2024-05-06T12:33:18Z

Have a look at the test_decomposition and test_quantum_control lit tests

@dime10 right, thanks! I have made these tests toml-independent. It required some further code reshaping, could you please review?

…test

frontend/catalyst/utils/toml.py

frontend/catalyst/qfunc.py

…test

frontend/catalyst/device/qjit_device.py

…test

dime10

Thanks I think this PR is almost good to go. The only thing I would ask to restrict the utils/toml module to be more focused on interacting with the toml file format, rather than including functionality related to device capabilities (it is a utility module after all).

frontend/catalyst/compiler.py

frontend/catalyst/jax_primitives.py

frontend/catalyst/third_party/cuda/primitives/__init__.py

dime10 · 2024-05-23T19:18:36Z

frontend/catalyst/utils/toml.py

+    return set(operations_no_adj)
+
+
+def validate_device_capabilities(


This module is just here to abstract away interacting with a TOML file right (and abstracting away from schema differences)? Validating device capabilities once the toml file has been processed then seems unrelated to interacting with the toml format. In that case I would prefer having this in the catalyst/device submodule, along with some of the other functions that don't interact with the toml file (I'm not 100% where the boundary should be the, probably anything that requires the device instance).

Done.
I personally think that it is not possible to find a perfect locations for code in Python+OOP style, so I am trying to focus on observable things only, like cyclic dependencies. The problem is in the fact that in this style we define types and methods simultaneously when we define classes (one per module), which forces some hierarchy. In practice, in contrast, we often need to process several equally-important types using a single algorithm. For my projects I (1) limit usage of "normal" classes (2) create types.py for all mypy types and dataclases (3) group functions in -py files trying to localize groups of cyclic deps among functions one per file.

What about get_device_toml_config and get_device_capabilities, their code seems to be operating primarily on the device, and then call into toml functions like read_toml_file and load_device_capabilities?

Co-authored-by: David Ittah <dime10@users.noreply.github.com>

…_device.py

…ode; Add a todo notice

dime10 · 2024-05-24T14:02:16Z

doc/changelog.md

+* Catalyst tests now manipulate device capabilities rather than text configurations files.
+  [(#712)](https://github.com/PennyLaneAI/catalyst/pull/712)


Reading this entry it sounds like it only concerns tests. Typically we exclude changelog entries for test-only, ci-only, doc-only, and repo-only changes.
Maybe the refactor can be mentioned in internal changes though? 🤔 New features, improvements, bug fixes should all be observable by the user in the distributed release in some way.

I don't think the commit changed anything?

dime10 · 2024-05-24T14:08:00Z

frontend/catalyst/utils/toml.py

+    return set(operations_no_adj)
+
+
+def validate_device_capabilities(


What about get_device_toml_config and get_device_capabilities, their code seems to be operating primarily on the device, and then call into toml functions like read_toml_file and load_device_capabilities?

**Context:** Catalyst needs to deal with operations which are not supported by the target device. This PR addresses decomposition-to-matrix branch. **Description of the Change:** We read gates to be decomposed to matrix from the device capabilities (which in turn are described in the device toml file). [sc-62011] **Benefits:** Device authors gain control over the decomposition strategies **Possible Drawbacks:** **Related GitHub Issues:** Requires #712 --------- Co-authored-by: lillian542 <38584660+lillian542@users.noreply.github.com>

Make a test tomle-schema-2-compatible

0ede63d

grwlf marked this pull request as ready for review May 3, 2024 12:22

grwlf requested a review from dime10 May 3, 2024 12:23

Address formatting issues

1babc99

grwlf self-assigned this May 3, 2024

grwlf changed the title ~~Make a test tomle-schema-2-compatible~~ Make decomposition test tomle-schema-2-compatible May 3, 2024

grwlf added 2 commits May 6, 2024 12:13

Make test toml-schema-independant

30de70c

Merge branch 'main' into toml-schema-2-update-test

9f985b0

grwlf added 3 commits May 6, 2024 12:15

Address codecov errors

f97e7e9

Address codecov errors

40499b4

Address codecov errors

0ed92b6

grwlf changed the title ~~Make decomposition test tomle-schema-2-compatible~~ [Frontend] Make tests tomle-schema independent May 6, 2024

grwlf changed the title ~~[Frontend] Make tests tomle-schema independent~~ [Frontend] Make tests toml-schema independent May 6, 2024

grwlf mentioned this pull request May 6, 2024

Update device toml specs to schema 2 PennyLaneAI/pennylane-lightning#642

Merged

grwlf added 2 commits May 9, 2024 09:18

Merge remote-tracking branch 'origin/main' into toml-schema-2-update-…

22cfe45

…test

Fix wrong field name

d00c7e4

grwlf mentioned this pull request May 9, 2024

[Frontend] Program verification #626

Open

5 tasks

dime10 reviewed May 9, 2024

View reviewed changes

frontend/catalyst/utils/toml.py Outdated Show resolved Hide resolved

frontend/catalyst/qfunc.py Outdated Show resolved Hide resolved

dime10 mentioned this pull request May 15, 2024

[Frontend] Fix decomposition tests #745

Open

grwlf added 2 commits May 16, 2024 11:48

Address review suggestions; Move code around

d3ffae5

Merge remote-tracking branch 'origin/main' into toml-schema-2-update-…

aa37716

…test

grwlf requested a review from dime10 May 16, 2024 12:09

grwlf added 3 commits May 16, 2024 12:16

Address formatting issues

6537b2a

Address formatting issues

468c3ca

Address pylint issues

72c9f24

Add missing paths module

611e5fc

lillian542 reviewed May 16, 2024

View reviewed changes

frontend/catalyst/device/qjit_device.py Show resolved Hide resolved

grwlf requested a review from lillian542 May 21, 2024 10:36

grwlf mentioned this pull request May 21, 2024

[Frontend] Pass capabilities to the decomposer #749

Merged

grwlf added 4 commits May 21, 2024 17:12

Merge branch 'main' into toml-schema-2-update-test

e94c55d

Fix re-added runtime.py

5bd08cb

Fix a test

f59e6bc

Merge remote-tracking branch 'origin/main' into toml-schema-2-update-…

8aadcf4

…test

dime10 reviewed May 23, 2024

View reviewed changes

grwlf and others added 7 commits May 24, 2024 13:59

Update frontend/catalyst/jax_primitives.py

1ba9149

Co-authored-by: David Ittah <dime10@users.noreply.github.com>

Update frontend/catalyst/third_party/cuda/primitives/__init__.py

4735b85

Co-authored-by: David Ittah <dime10@users.noreply.github.com>

Rename paths -> runtime_environment

74ec4ff

Address review suggestions: move validate_device_requirements -> qjit…

8793214

…_device.py

Address review suggestions: remove self.qjit_device attribute from qn…

04a5b41

…ode; Add a todo notice

Fix a test import

2bb3ede

Merge branch 'main' into toml-schema-2-update-test

722886c

grwlf requested a review from dime10 May 24, 2024 11:57

Update changelog

eadeb91

dime10 approved these changes May 24, 2024

View reviewed changes

grwlf merged commit 715523e into main May 24, 2024
41 of 42 checks passed

grwlf deleted the toml-schema-2-update-test branch May 24, 2024 14:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Frontend] Make tests toml-schema independent #712

[Frontend] Make tests toml-schema independent #712

grwlf commented May 3, 2024 •

edited

dime10 commented May 3, 2024

grwlf commented May 3, 2024

dime10 commented May 3, 2024

codecov bot commented May 6, 2024 •

edited

grwlf commented May 6, 2024 •

edited

dime10 left a comment

dime10 May 23, 2024

grwlf May 24, 2024 •

edited

dime10 May 24, 2024 •

edited

grwlf May 24, 2024

dime10 May 24, 2024

dime10 May 24, 2024 •

edited

grwlf May 24, 2024

dime10 May 24, 2024

dime10 May 24, 2024 •

edited

		return set(operations_no_adj)


		def validate_device_capabilities(

		* Catalyst tests now manipulate device capabilities rather than text configurations files.
		[(#712)](https://github.com/PennyLaneAI/catalyst/pull/712)

[Frontend] Make tests toml-schema independent #712

[Frontend] Make tests toml-schema independent #712

Conversation

grwlf commented May 3, 2024 • edited

dime10 commented May 3, 2024

grwlf commented May 3, 2024

dime10 commented May 3, 2024

codecov bot commented May 6, 2024 • edited

Codecov Report

grwlf commented May 6, 2024 • edited

dime10 left a comment

Choose a reason for hiding this comment

dime10 May 23, 2024

Choose a reason for hiding this comment

grwlf May 24, 2024 • edited

Choose a reason for hiding this comment

dime10 May 24, 2024 • edited

Choose a reason for hiding this comment

grwlf May 24, 2024

Choose a reason for hiding this comment

dime10 May 24, 2024

Choose a reason for hiding this comment

dime10 May 24, 2024 • edited

Choose a reason for hiding this comment

grwlf May 24, 2024

Choose a reason for hiding this comment

dime10 May 24, 2024

Choose a reason for hiding this comment

dime10 May 24, 2024 • edited

Choose a reason for hiding this comment

grwlf commented May 3, 2024 •

edited

codecov bot commented May 6, 2024 •

edited

grwlf commented May 6, 2024 •

edited

grwlf May 24, 2024 •

edited

dime10 May 24, 2024 •

edited

dime10 May 24, 2024 •

edited

dime10 May 24, 2024 •

edited