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

Remove qpe_hubbard tests #849

Closed
wants to merge 63 commits into from
Closed

Conversation

Epsilon1024
Copy link
Contributor

These tests are very large and will cause problems when running. These
tests are now disabled.

Epsilon1024 and others added 24 commits February 14, 2024 01:49
unit tests to ensure round trip of each datatype using both numeric and
sympy inputs.
Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
These tests are very large and will cause problems when running. These
tests are now disabled.
@mpharrigan
Copy link
Collaborator

def test_whatever(bloq_autotester):
  if bloq_autotester.check_name == 'serialization':
    pytest.skip('too big')

  bloq_autotester(_whatever_example)

Epsilon1024 added a commit to Epsilon1024/Qualtran that referenced this pull request Apr 4, 2024
Rather than building on top of quantumlib#849, we keep the changes separate.
@Epsilon1024
Copy link
Contributor Author

def test_whatever(bloq_autotester):
  if bloq_autotester.check_name == 'serialization':
    pytest.skip('too big')

  bloq_autotester(_whatever_example)

I gave this a try using test_qubitization_qpe_bloq_autotester as the "test_whatever" method. I noticed two issues with this:

  1. test_qubitization_qpe_bloq_autotester is never called from get_bloq_report_card. The stack goes from get_bloq_report_card -> record_for_bloq_example ->check_bloq_example_serialize->assert_bloq_example_serialize

It looks like this won't prevent this test from running.

  1. The other thing I noticed is that _qubitization_qpe_hubbard_model_large doesn't seem to be called by an autotester. However, _qubitization_qpe_hubbard_model_small is called by an autotester

@mpharrigan
Copy link
Collaborator

get_bloq_report_card is called ad-hoc by the developer outside of the CI flow. Re: (2) you have to "opt in" to including the auto-tests as part of the pytest unit tests, which is run as part of the CI flow

@Epsilon1024
Copy link
Contributor Author

Epsilon1024 commented Apr 5, 2024

get_bloq_report_card is called ad-hoc by the developer outside of the CI flow. Re: (2) you have to "opt in" to including the auto-tests as part of the pytest unit tests, which is run as part of the CI flow

Gotcha. Should we address how to remove it in get_bloq_report_card as well?

Also, it looks like only _qubitization_qpe_hubbard_model_small (and not the large version) is run on the pytest. While the small model may be a bit inconvenient to run locally, it should be small enough to run on the workflow. If that is the case, we might not have to do anything and should revert the changes in this PR.

tanujkhattar added a commit that referenced this pull request Apr 19, 2024
* Added methods to serialize and deserialize quantum data types.

* Add serialization and deserialization methods for all data types. Write
unit tests to ensure round trip of each datatype using both numeric and
sympy inputs.

* Update qualtran/protos/data_types.proto

Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>

* Update Proto:wq

* Fixed error formatting

* Remove bitsize from QBit

* Fixed formatting

* Remove uncessesary files

* Fix data type in formatted string in error message

* Update qualtran/serialization/data_types_test.py

* Added Sympy serialization.

Sympy expressions can now be serialized through a recursive proto.

* Clean up workspace

* Remote large tests from get_bloq_examples()

* Remove qpe_hubbard_model tests

The qpe_hubbard_model is too large to test and causes developer
friction. We have disabled the tests for now

* Remove qpe_hubbard tests

These tests are very large and will cause problems when running. These
tests are now disabled.

* Cleanup branch

* Only remove the serialization tests from the large bloq.s

Move the changes from bloq_finder.py to bloq_report_card.py.  This way,
only the serialization portion of the test is affected.

* Address issues with failing sympy tests.

Added support for fraction constant parameters.
Fixed issue with cwap test assertion which expected sympy in the old
string format.

* Fix formatting

* Fix formatting

* Delete Untitled.ipynb

Delete scratch notebook

* Cleanup after merging local branches.

(Old changes were accidentally left in).

* Remove changes from PR#849

Rather than building on top of #849, we keep the changes separate.

* Fixed assortment of bugs in fraction, and constant symbol serialization

* Scratch test

* Fixed bad reference to resolver dict by bloq module name.

* Minor refactoring

* Ran formatting tools

* Cleanup forgotten test code.

* Add tests, fix naming conventions, and add proper return types.

* Update sympy_test.py

Call sympy serialization directly rather than calling it through bloq.

* Apply suggestions from code review

Addressed nit comments.

Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>

* Fix sympy_test

* Fix return type of sympy_expr_from_proto

---------

Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
@Epsilon1024
Copy link
Contributor Author

I don't think these changes are needed any longer. We can close this PR.

@Epsilon1024 Epsilon1024 closed this May 3, 2024
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