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

Trmorgan/keyboard interrupt #6192

Merged
merged 17 commits into from
Jul 4, 2024

Conversation

trevormorgan
Copy link
Contributor

@trevormorgan trevormorgan commented Jun 21, 2024

Improved Interrupt Handling in dond Function

Description

This PR enhances the interrupt handling capabilities of the dond function in the QCoDeS dataset module. It addresses issues with the current implementation of catch_interrupts and ensures that keyboard interrupts are properly propagated and handled during measurement sweeps.

New

  • Ensured that KeyboardInterrupt is properly raised and propagated through the dond function

Testing

The updated test now correctly simulates a KeyboardInterrupt within the dond function and verifies that it's propagated as expected. This change improves our ability to test and ensure robust interrupt handling in complex measurement scenarios.

Impact

These changes improve the reliability of measurements that use the dond function, particularly in situations where users might need to interrupt long-running sweeps. It ensures that the system responds appropriately to keyboard interrupts, possible saving time due to prior option being complete abort

Additional Notes

  • This PR does not introduce any changes to the core dond function itself, only to its testing infrastructure
  • Users should not experience any changes in normal operation, but will benefit from improved interrupt handling

Trevor Morgan added 9 commits June 12, 2024 14:49
…atch_interrupts` function.

# Aider chat conversation:

USER: check the keyboard interupt code for being able to abort a loop of dond sweeps. Create unit tests with pytest to validate this function

ASSISTANT: To validate the keyboard interrupt functionality for aborting a loop of `dond` sweeps, we need to:

1. Ensure the existing code properly handles keyboard interrupts.
2. Create unit tests using `pytest` to validate this functionality.

First, let's review the existing code for handling keyboard interrupts in `src\qcodes\dataset\dond\do_nd_utils.py`.

Next, we'll create a new test file `tests\dataset\dond\test_interrupts.py` to validate the interrupt functionality.

Here are the changes:

### Review and Ensure Keyboard Interrupt Handling

src\qcodes\dataset\dond\do_nd_utils.py
```python
<<<<<<< SEARCH
@contextmanager
def catch_interrupts() -> Iterator[Callable[[], MeasInterruptT | None]]:
    interrupt_exception: MeasInterruptT | None = None

    def get_interrupt_exception() -> MeasInterruptT | None:
        nonlocal interrupt_exception
        return interrupt_exception

    try:
        yield get_interrupt_exception
    except (KeyboardInterrupt, BreakConditionInterrupt) as e:
        interrupt_exception = e
        raise
=======
@contextmanager
def catch_interrupts() -> Iterator[Callable[[], MeasInterruptT | None]]:
    interrupt_exception: MeasInterruptT | None = None

    def get_interrupt_exception() -> MeasInterruptT | None:
        nonlocal interrupt_exception
        return interrupt_exception

    try:
        yield get_interrupt_exception
    except (KeyboardInterrupt, BreakConditionInterrupt) as e:
        interrupt_exception = e
        raise
>>>>>>> REPLACE
```

### Create Unit Tests

tests\dataset\dond\test_interrupts.py
```python
<<<<<<< SEARCH
=======
import pytest
from qcodes.dataset.dond.do_nd_utils import catch_interrupts, BreakConditionInterrupt

def test_keyboard_interrupt_handling():
    with pytest.raises(KeyboardInterrupt):
        with catch_interrupts() as get_interrupt_exception:
            raise KeyboardInterrupt
        assert isinstance(get_interrupt_exception(), KeyboardInterrupt)

def test_break_condition_interrupt_handling():
    with pytest.raises(BreakConditionInterrupt):
        with catch_interrupts() as get_interrupt_exception:
            raise BreakConditionInterrupt
        assert isinstance(get_interrupt_exception(), BreakConditionInterrupt)

def test_no_interrupt_handling():
    with catch_interrupts() as get_interrupt_exception:
        pass
    assert get_interrupt_exception() is None
>>>>>>> REPLACE
```

These changes ensure that the keyboard interrupt handling is properly validated with unit tests.
@trevormorgan trevormorgan requested a review from a team as a code owner June 21, 2024 20:00
Copy link

codecov bot commented Jun 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.18%. Comparing base (009ec80) to head (824152d).
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6192   +/-   ##
=======================================
  Coverage   67.17%   67.18%           
=======================================
  Files         352      352           
  Lines       32132    32140    +8     
=======================================
+ Hits        21586    21594    +8     
  Misses      10546    10546           

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

@trevormorgan trevormorgan added this to the 0.46.0 milestone Jul 4, 2024
@trevormorgan trevormorgan self-assigned this Jul 4, 2024
@jenshnielsen jenshnielsen added this pull request to the merge queue Jul 4, 2024
Merged via the queue into microsoft:main with commit 35500b2 Jul 4, 2024
17 checks passed
jenshnielsen added a commit to jenshnielsen/Qcodes that referenced this pull request Jul 4, 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.

2 participants