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

Simulated vs Dummy devices #307

Open
juliomateoslangerak opened this issue Aug 16, 2024 · 7 comments
Open

Simulated vs Dummy devices #307

juliomateoslangerak opened this issue Aug 16, 2024 · 7 comments
Labels

Comments

@juliomateoslangerak
Copy link
Contributor

I wanted to write some test code for the SLM and realized that there are dummy devices for test, where I thought the simulated devices were going to be used.
What is the sense of keeping both types of devices? What are the differences?

@carandraug
Copy link
Collaborator

New stuff should got into microscope.simulators. There is also stuff in microscope.testsuite.devices, mainly stuff imported from microscope.simulators under a different name and that is done for backwards compatibility.

Special exceptions are DummyDSP and DummySLM which have not been moved to microscope.simulators because microscope doesn't actually have classes for them (maybe they shouldn't have been moved to microscope in the first place).

@juliomateoslangerak
Copy link
Contributor Author

Thanks. So how do I test a device. Create a DummyX imported from SimulatedX or I just use SimulatedX in the test.

Could I replace DummySLM? Or is it required to still be there for testing not yet legacy implementations of the SLM in cockpit?

@iandobbie
Copy link
Collaborator

Special exceptions are DummyDSP and DummySLM which have not been moved to microscope.simulators because microscope doesn't actually have classes for them (maybe they shouldn't have been moved to microscope in the first place).

SLM definitely should be in microscope, it was always planned but we never got around to doing the actual work, which Julio has now done (thanks). We have had discussions about where the dsp should sit (or I guess it should now be called executor). I don't think we ever came to a concrete conclusion and it has always been rather orphaned of by its self. Happy to revisit that discussion but only seems sensible if someone has time to actually implement something, say the pi pico digital only executor. Its on my todo list but keeps not getting done.

@carandraug
Copy link
Collaborator

@iandobbie writes:

Special exceptions are DummyDSP and DummySLM [...] (maybe they shouldn't have been moved to microscope in the first place).

SLM definitely should be in microscope, [...]

An interface to SLMs should be in Microscope but maybe not the DummySLM we have now which implements an interface that we now we don't want to in Microscope and that is only used by Cockpit.

@juliomateoslangerak writes:

Thanks. So how do I test a device. Create a DummyX imported from SimulatedX or I just use SimulatedX in the test.

Could I replace DummySLM? Or is it required to still be there for testing not yet legacy implementations of the SLM in cockpit?

This is about adding SLM support to Microscope so do this:

  1. define the new ABC in microscope.abc
  2. implement a new simulated device in microscope.simulators
  3. implement other concrete classes for real hardware in microscope.slm
  4. add tests for the new simulated device in microscope.testsuite.test_devices
  5. if you need a separate test device that will be used in the Microscope testsuite only then add it to microscope.testsuite.devices (you probably don't need this)
  6. Leave microscope.testsuite.devices.DummySLM there. There is a comment on it saying "This only exists to test cockpit. There is no corresponding device type in microscope yet.". Add a note there saying that this class implements the original SLM interface that was never part of Python-Microscope but that is used by cockpit for testing and that it should be removed once Cockpit has adopted the new SLM interface.

@juliomateoslangerak
Copy link
Contributor Author

I went until step 5 but, the problem seems to me that test_devices.py defines:

class TestDummySLM(unittest.TestCase, SLMTests):
    def setUp(self):
        self.device = dummies.DummySLM()

where SLMTests is a class that just passes, so the test is just testing instantiation of the legacy DummySLM and the basic tests from parent DeviceTests. If I keep the same pattern, following the DM example, and add the tests under SLMTests, the "TestDummySLM" will fail.

One solution I see is to refactor current TestDummyXXX to TestSimulatedXXX and keep the TestDummySLM and TestDummyDSP as they are now but inheriting directly from DeviceTests. There I could easily add the TestSimulatedSLM.

Does that seem a solution to you?

One question a bit off topic.
I see there are a bunch of mocked devices. Does it add a lot of value to mock devices for the required (maintenance) effort? I don't really see much for the SLM.

@carandraug
Copy link
Collaborator

One solution I see is to refactor current TestDummyXXX to TestSimulatedXXX and keep the TestDummySLM and TestDummyDSP as they are now but inheriting directly from DeviceTests. There I could easily add the TestSimulatedSLM.

Does that seem a solution to you?

That would work. But as you pointed out, we don't really have tests for the DummySLM ad the goal is to remove it when we do have a SLM class. So keep things simpler in the future, just change TestDummySLM to subclass from DeviceTests and not SLMTests. Then add whatever you need to SLMTests.

One question a bit off topic.
I see there are a bunch of mocked devices. Does it add a lot of value to mock devices for the required (maintenance) effort? I don't really see much for the SLM.

mock devices should not require maintenance effort. They should be done once and that's it. I think the ones we currently have were too much work to implement and we need something simpler. If you can think of something better for you, then that would be nice but we won't block. The problem is that without mock devices, it's very difficult to make changes that affect them (and changes to the ABCs can affect all the devices).

But we don't have mock devices for most devices so it's up to you if you want to do it.

@juliomateoslangerak
Copy link
Contributor Author

juliomateoslangerak commented Aug 21, 2024

So keep things simpler in the future, just change TestDummySLM to subclass from DeviceTests and not SLMTests. Then add whatever you need to SLMTests.

Yes, but tests in SLMTests will not be run unless subclassed. So I have to create a TestSimulatedSLM subclassing SLMTests. So the question is more about naming consistency, as most Tests are using simulated devices, I would refactor those to be more clear. As an alternative, I could refactor the current SLM test to TestDummyLegacySLM and point here in the docstring and keep my SLM test as TestDummySLM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants