From b23aca5445473a02540feea976f297dd800f2005 Mon Sep 17 00:00:00 2001 From: Giulio Ungaretti Date: Mon, 29 Aug 2016 18:43:34 +0200 Subject: [PATCH] Feat: Default to no multiprocessing. Because multiprocessing is not open for public use, unless one really wants to use it. The test are incredibly coupled, so they needed some massaging too. Closes #314. --- .travis.yml | 4 ++-- qcodes/data/data_set.py | 34 +++++++++++++++------------- qcodes/instrument/base.py | 8 ++++--- qcodes/instrument/remote.py | 1 - qcodes/loops.py | 18 ++++++++++----- qcodes/process/helpers.py | 2 ++ qcodes/process/stream_queue.py | 3 ++- qcodes/tests/test_data.py | 8 +++---- qcodes/tests/test_driver_testcase.py | 2 +- qcodes/tests/test_instrument.py | 10 ++++---- qcodes/tests/test_loop.py | 28 +++++++++++++++-------- qcodes/tests/test_visa.py | 2 +- 12 files changed, 72 insertions(+), 48 deletions(-) diff --git a/.travis.yml b/.travis.yml index 515345b14805..b6645af1d70e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,8 +18,8 @@ install: - python setup.py develop # command to run tests script: - - python qcodes/test.py --skip-coverage - - python qcodes/test.py --mp-spawn + - python qcodes/test.py --skip-coverage -f + - python qcodes/test.py --mp-spawn -f after_success: # install codacy coverage plugin only on traivs - pip install codacy-coverage diff --git a/qcodes/data/data_set.py b/qcodes/data/data_set.py index 57d65093fefd..fd99cdf4c360 100644 --- a/qcodes/data/data_set.py +++ b/qcodes/data/data_set.py @@ -27,7 +27,9 @@ class DataMode(Enum): def new_data(location=None, loc_record=None, name=None, overwrite=False, - io=None, data_manager=None, mode=DataMode.LOCAL, **kwargs): + io=None, data_manager=False, mode=DataMode.LOCAL, **kwargs): + # NOTE(giulioungaretti): leave this docstrings as it is, because + # documenting the types is silly in this case. """ Create a new DataSet. @@ -57,11 +59,11 @@ def new_data(location=None, loc_record=None, name=None, overwrite=False, says the root data directory is the current working directory, ie where you started the python session. - data_manager (DataManager or False, optional): manager for the - ``DataServer`` that offloads storage and syncing of this - ``DataSet``. Usually omitted (default None) to use the default - from ``get_data_manager()``. If ``False``, this ``DataSet`` will - store itself. + data_manager (Optional[bool]): use a manager for the + ``DataServer`` that offloads storage and syncing of this Defaults + to ``False`` i.e. this ``DataSet`` will store itself without extra + processes. ``DataSet``. Set to ``True`` to use the default from + ``get_data_manager()``. mode (DataMode, optional): connection type to the ``DataServer``. ``DataMode.LOCAL``: this DataSet doesn't communicate across @@ -105,11 +107,11 @@ def new_data(location=None, loc_record=None, name=None, overwrite=False, if location and (not overwrite) and io.list(location): raise FileExistsError('"' + location + '" already has data') - if data_manager is False: + if data_manager is True: + data_manager = get_data_manager() + else: if mode != DataMode.LOCAL: raise ValueError('DataSets without a data_manager must be local') - elif data_manager is None: - data_manager = get_data_manager() return DataSet(location=location, io=io, data_manager=data_manager, mode=mode, **kwargs) @@ -206,11 +208,11 @@ class DataSet(DelegateAttributes): says the root data directory is the current working directory, ie where you started the python session. - data_manager (DataManager or False, optional): manager for the - ``DataServer`` that offloads storage and syncing of this - ``DataSet``. Usually omitted (default None) to use the default - from ``get_data_manager()``. If ``False``, this ``DataSet`` will - store itself. + data_manager (Optional[bool]): use a manager for the + ``DataServer`` that offloads storage and syncing of this Defaults + to ``False`` i.e. this ``DataSet`` will store itself without extra + processes. ``DataSet``. Set to ``True`` to use the default from + ``get_data_manager()``. mode (DataMode, optional): connection type to the ``DataServer``. ``DataMode.LOCAL``: this DataSet doesn't communicate across @@ -258,7 +260,7 @@ class DataSet(DelegateAttributes): background_functions = OrderedDict() def __init__(self, location=None, mode=DataMode.LOCAL, arrays=None, - data_manager=None, formatter=None, io=None, write_period=5): + data_manager=False, formatter=None, io=None, write_period=5): if location is False or isinstance(location, str): self.location = location else: @@ -281,7 +283,7 @@ def __init__(self, location=None, mode=DataMode.LOCAL, arrays=None, for array in arrays: self.add_array(array) - if data_manager is None and mode in SERVER_MODES: + if data_manager is True and mode in SERVER_MODES: data_manager = get_data_manager() if mode == DataMode.LOCAL: diff --git a/qcodes/instrument/base.py b/qcodes/instrument/base.py index d12c713fee22..33691ca12d93 100644 --- a/qcodes/instrument/base.py +++ b/qcodes/instrument/base.py @@ -1,7 +1,8 @@ """Instrument base class.""" -import weakref -import time import logging +import time +import warnings +import weakref from qcodes.utils.metadata import Metadatable from qcodes.utils.helpers import DelegateAttributes, strip_attrs, full_class @@ -75,11 +76,12 @@ class Instrument(Metadatable, DelegateAttributes, NestedAttrAccess): _all_instruments = {} - def __new__(cls, *args, server_name='', **kwargs): + def __new__(cls, *args, server_name=None, **kwargs): """Figure out whether to create a base instrument or proxy.""" if server_name is None: return super().__new__(cls) else: + warnings.warn("Multiprocessing is in beta, use at own risk", UserWarning) return RemoteInstrument(*args, instrument_class=cls, server_name=server_name, **kwargs) diff --git a/qcodes/instrument/remote.py b/qcodes/instrument/remote.py index d65409845560..f5e133e8160a 100644 --- a/qcodes/instrument/remote.py +++ b/qcodes/instrument/remote.py @@ -43,7 +43,6 @@ class RemoteInstrument(DelegateAttributes): def __init__(self, *args, instrument_class=None, server_name='', **kwargs): - if server_name == '': server_name = instrument_class.default_server_name(**kwargs) diff --git a/qcodes/loops.py b/qcodes/loops.py index e4b89c3bf799..3a5e020f6c44 100644 --- a/qcodes/loops.py +++ b/qcodes/loops.py @@ -50,6 +50,7 @@ import multiprocessing as mp import time import numpy as np +import warnings from qcodes.station import Station from qcodes.data.data_set import new_data, DataMode @@ -62,7 +63,10 @@ from .actions import (_actions_snapshot, Task, Wait, _Measure, _Nest, BreakIf, _QcodesBreak) +# Switches off multiprocessing by default, cant' be altered after module import. +# TODO(giulioungaretti) use config. +USE_MP = False MP_NAME = 'Measurement' @@ -640,7 +644,7 @@ def _check_signal(self): else: raise ValueError('unknown signal', signal_) - def get_data_set(self, data_manager=None, *args, **kwargs): + def get_data_set(self, data_manager=False, *args, **kwargs): """ Return the data set for this loop. If no data set has been created yet, a new one will be created and returned. @@ -670,6 +674,7 @@ def get_data_set(self, data_manager=None, *args, **kwargs): if data_manager is False: data_mode = DataMode.LOCAL else: + warnings.warn("Multiprocessing is in beta, use at own risk", UserWarning) data_mode = DataMode.PUSH_TO_SERVER data_set = new_data(arrays=self.containers(), mode=data_mode, @@ -688,20 +693,19 @@ def run_temp(self, **kwargs): return self.run(background=False, quiet=True, data_manager=False, location=False, **kwargs) - def run(self, background=True, use_threads=True, quiet=False, - data_manager=None, station=None, progress_interval=False, + def run(self, background=USE_MP, use_threads=False, quiet=False, + data_manager=USE_MP, station=None, progress_interval=False, *args, **kwargs): """ Execute this loop. - background: (default True) run this sweep in a separate process + background: (default False) run this sweep in a separate process so we can have live plotting and other analysis in the main process use_threads: (default True): whenever there are multiple `get` calls back-to-back, execute them in separate threads so they run in parallel (as long as they don't block each other) quiet: (default False): set True to not print anything except errors - data_manager: a DataManager instance (omit to use default, - False to store locally) + data_manager: set to True to use a DataManager. Default to False. station: a Station instance for snapshots (omit to use a previously provided Station, or the default Station) progress_interval (default None): show progress of the loop every x @@ -737,6 +741,7 @@ def run(self, background=True, use_threads=True, quiet=False, prev_loop.join() data_set = self.get_data_set(data_manager, *args, **kwargs) + self.set_common_attrs(data_set=data_set, use_threads=use_threads, signal_queue=self.signal_queue) @@ -762,6 +767,7 @@ def run(self, background=True, use_threads=True, quiet=False, flush=True) if background: + warnings.warn("Multiprocessing is in beta, use at own risk", UserWarning) p = QcodesProcess(target=self._run_wrapper, name=MP_NAME) p.is_sweep = True p.signal_queue = self.signal_queue diff --git a/qcodes/process/helpers.py b/qcodes/process/helpers.py index 7c613bbed8e8..25a91f3bea96 100644 --- a/qcodes/process/helpers.py +++ b/qcodes/process/helpers.py @@ -2,6 +2,7 @@ import multiprocessing as mp import time +import warnings MP_ERR = 'context has already been set' @@ -23,6 +24,7 @@ def set_mp_method(method, force=False): with the *same* method raises an error, but here we only raise the error if you *don't* force *and* the context changes """ + warnings.warn("Multiprocessing is in beta, use at own risk", UserWarning) try: mp.set_start_method(method, force=force) except RuntimeError as err: diff --git a/qcodes/process/stream_queue.py b/qcodes/process/stream_queue.py index 86e2f3d675d9..3dcbc5bff62a 100644 --- a/qcodes/process/stream_queue.py +++ b/qcodes/process/stream_queue.py @@ -2,9 +2,10 @@ import multiprocessing as mp import sys -from datetime import datetime import time +from datetime import datetime + from .helpers import kill_queue diff --git a/qcodes/tests/test_data.py b/qcodes/tests/test_data.py index 0afe605af11c..b49131f4e6e0 100644 --- a/qcodes/tests/test_data.py +++ b/qcodes/tests/test_data.py @@ -449,14 +449,14 @@ def test_from_server(self, gdm_mock): mock_dm.live_data = MockLive() # wrong location or False location - converts to local - data = DataSet(location='Jupiter', mode=DataMode.PULL_FROM_SERVER) + data = DataSet(location='Jupiter', data_manager=True, mode=DataMode.PULL_FROM_SERVER) self.assertEqual(data.mode, DataMode.LOCAL) - data = DataSet(location=False, mode=DataMode.PULL_FROM_SERVER) + data = DataSet(location=False, data_manager=True, mode=DataMode.PULL_FROM_SERVER) self.assertEqual(data.mode, DataMode.LOCAL) # location matching server - stays in server mode - data = DataSet(location='Mars', mode=DataMode.PULL_FROM_SERVER, + data = DataSet(location='Mars', data_manager=True, mode=DataMode.PULL_FROM_SERVER, formatter=MockFormatter()) self.assertEqual(data.mode, DataMode.PULL_FROM_SERVER) self.assertEqual(data.arrays, MockLive.arrays) @@ -495,7 +495,7 @@ def test_to_server(self, gdm_mock): mock_dm.needs_restart = True gdm_mock.return_value = mock_dm - data = DataSet(location='Venus', mode=DataMode.PUSH_TO_SERVER) + data = DataSet(location='Venus', data_manager=True, mode=DataMode.PUSH_TO_SERVER) self.assertEqual(mock_dm.needs_restart, False, data) self.assertEqual(mock_dm.data_set, data) self.assertEqual(data.data_manager, mock_dm) diff --git a/qcodes/tests/test_driver_testcase.py b/qcodes/tests/test_driver_testcase.py index 185c0e07559b..b91b04411c5a 100644 --- a/qcodes/tests/test_driver_testcase.py +++ b/qcodes/tests/test_driver_testcase.py @@ -34,7 +34,7 @@ class TestDriverTestCase(DriverTestCase): @classmethod def setUpClass(cls): cls.an_empty_model = EmptyModel() - cls.an_instrument = MockMock('a', model=cls.an_empty_model) + cls.an_instrument = MockMock('a', model=cls.an_empty_model, server_name='') super().setUpClass() @classmethod diff --git a/qcodes/tests/test_instrument.py b/qcodes/tests/test_instrument.py index 7299c1c9bad8..115776b22b4a 100644 --- a/qcodes/tests/test_instrument.py +++ b/qcodes/tests/test_instrument.py @@ -51,11 +51,13 @@ class TestInstrument(TestCase): def setUpClass(cls): cls.model = AMockModel() - cls.gates = MockGates(model=cls.model) - cls.source = MockSource(model=cls.model) - cls.meter = MockMeter(model=cls.model, keep_history=False) + import pdb + cls.gates = MockGates(model=cls.model, server_name='') + cls.source = MockSource(model=cls.model, server_name='') + cls.meter = MockMeter(model=cls.model, keep_history=False, server_name='') def setUp(self): + import pdb # reset the model state via the gates function self.gates.reset() @@ -192,7 +194,7 @@ def test_remove_instance(self): with self.assertRaises(KeyError): Instrument.find_instrument('gates') - type(self).gates = MockGates(model=self.model) + type(self).gates = MockGates(model=self.model, server_name="") self.assertEqual(self.gates.instances(), [self.gates]) self.assertEqual(Instrument.find_instrument('gates'), self.gates) diff --git a/qcodes/tests/test_loop.py b/qcodes/tests/test_loop.py index 2ae8ecd0fa55..d13d84b930a2 100644 --- a/qcodes/tests/test_loop.py +++ b/qcodes/tests/test_loop.py @@ -33,9 +33,9 @@ def setUp(self): self.model = AMockModel() - self.gates = MockGates(model=self.model) - self.source = MockSource(model=self.model) - self.meter = MockMeter(model=self.model) + self.gates = MockGates(model=self.model, server_name='') + self.source = MockSource(model=self.model, server_name='') + self.meter = MockMeter(model=self.model, server_name='') self.location = '_loop_test_' self.location2 = '_loop_test2_' self.io = DiskIO('.') @@ -78,7 +78,7 @@ def test_background_and_datamanager(self): # you can only do in-memory loops if you set data_manager=False # TODO: this is the one place we don't do quiet=True - test that we # really print stuff? - data = self.loop.run(location=self.location) + data = self.loop.run(location=self.location, background=True) self.check_empty_data(data) # wait for process to finish (ensures that this was run in the bg, @@ -100,7 +100,9 @@ def test_local_instrument(self): # if spawn, pickle will happen if mp.get_start_method() == "spawn": with self.assertRaises(RuntimeError): - loop_local.run(location=self.location, quiet=True) + loop_local.run(location=self.location, + quiet=True, + background=True) # allow for *nix # TODO(giulioungaretti) see what happens ? # what is the expected beavhiour ? @@ -115,7 +117,9 @@ def test_local_instrument(self): self.check_loop_data(data) def test_background_no_datamanager(self): - data = self.loop.run(location=self.location, data_manager=False, + data = self.loop.run(location=self.location, + background=True, + data_manager=False, quiet=True) self.check_empty_data(data) @@ -168,12 +172,18 @@ def test_foreground_no_datamanager(self): def test_enqueue(self): c1 = self.gates.chan1 loop = Loop(c1[1:5:1], 0.01).each(c1) - data1 = loop.run(location=self.location, quiet=True) + data1 = loop.run(location=self.location, + quiet=True, + background=True, + data_manager=True) # second running of the loop should be enqueued, blocks until # the first one finishes. # TODO: check what it prints? - data2 = loop.run(location=self.location2, quiet=True) + data2 = loop.run(location=self.location2, + quiet=True, + background=True, + data_manager=True) data1.sync() data2.sync() @@ -646,7 +656,7 @@ def g(): }, 'loop': { 'background': False, - 'use_threads': True, + 'use_threads': False, 'use_data_manager': False, '__class__': 'qcodes.loops.ActiveLoop', 'sweep_values': { diff --git a/qcodes/tests/test_visa.py b/qcodes/tests/test_visa.py index c09e82f5ea8c..93c56321482f 100644 --- a/qcodes/tests/test_visa.py +++ b/qcodes/tests/test_visa.py @@ -118,7 +118,7 @@ def test_ask_write_local(self): def test_ask_write_server(self): # same thing as above but Joe is on a server now... - mv = MockVisa('Joe') + mv = MockVisa('Joe', server_name='') # test normal ask and write behavior mv.state.set(2)