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

Pulse unittest reorganization #6106

Open
6 of 18 tasks
nkanazawa1989 opened this issue Mar 29, 2021 · 1 comment
Open
6 of 18 tasks

Pulse unittest reorganization #6106

nkanazawa1989 opened this issue Mar 29, 2021 · 1 comment
Labels
type: enhancement It's working, but needs polishing

Comments

@nkanazawa1989
Copy link
Contributor

nkanazawa1989 commented Mar 29, 2021

What is the expected enhancement?

With introduction of pulse ScheduleBlock (see #5679), the software implementation of the qiskit pulse was drastically updated while minimizing the impact to user API. However, the unittests are still based on the old codebase structure and there are many redundant/duplicated tests.

Basic changes based on the ScheduleBlock are listed as follows:

  • ScheduleBlock is added as the output program format of the builder syntax.
  • pulse.transforms turned into a package, aiming at pass manager implementation.
    • Alignment transforms became a class instance rather than callbacks to keep arguments attached to the macro.
  • Parameter framework is replaced with the visitor pattern (see discussion)
    • Parameter relevant test for any program representation can be tested in a single place now.

I suggest to reorganize test.python.pulse as follows:

  • pulse_test_utils.py
    • A common 2Q backend that can be used for unittest of ScheduleBlock, Schedule and builder syntax.
      • Some easy integration test model from backend.defaults input, program generation to Qobj generation.
    • A collection of some util methods.
  • test_block.py
    • Remove tests for parameters
    • Remove tests for alignment context
    • More scalable test with the common backend
  • test_builder.py
    • Rewrite reference schedule in the block representation (since output is block)
    • Remove tests for parameters
    • Remove tests for alignment context
    • More scalable test with the common backend
  • test_channels.py (as-is)
  • test_library.py
    • Merge test_continuous_pulses.py, test_discrete_pulses.py, test_samplers.py, and test_pulse_lib.py
  • test_continuous_pulses.py
    • Remove
  • test_discrete_pulses.py
    • Remove
  • test_experiment_configurations.py (as-is)
  • test_instruction_schedule_map.py (as-is)
  • test_instructions.py (as-is)
  • test_macros.py (as-is)
  • test_parameter_manager.py
    • Add tests for parametrized Schedule (this is indirectly tested with parametrized builder syntax test)
    • Add explicit test for each instruction
  • test_parameters.py
    • Remove (this is old parameter framework, just supported for backward compatibility)
  • test_parser.py (as-is)
  • test_pulse_lib.py
    • Remove
  • test_samplers.py
    • Remove
  • test_schedule.py
    • More scalable test with the common backend
  • test_transforms.py
    • Migrate alignment relevant tests from test_builder.py and test_block.py
    • Replace function based alignment with AlignmentKind class

We also need further discussion for granularity of unittest classes. These classes are prepared for a specific method, collection of methods (some behavior), or subclass-wise, i.e. seems no standard rule. This may make us easily miss important testcases and raise the bar for contribution. Perhaps need some guideline? Welcome suggestions :)

@nkanazawa1989 nkanazawa1989 added the type: enhancement It's working, but needs polishing label Mar 29, 2021
@lcapelluto
Copy link
Contributor

sounds good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement It's working, but needs polishing
Projects
None yet
Development

No branches or pull requests

2 participants