Fix the remaining tests for py37 #166

justheuristic · 2021-03-01T21:50:53Z

DecentralizedAverager is now compatible with python37's acyncio exception
- the problem was: grpc.aio with python37 raised concurrent.futures.CancelledError in some cases;
- we relied on isinstance(asyncio.CancelledError, Exception) == False
- but isinstance(concurrent.futures.CancelledError, Exception) == True
DecentralizedAverager now shuts down if dereferenced in the main process
- though it won't shutdown if dereferenced in forks for obvious reasons
HIVEMIND_THREADS now actually works
test_averaging now shuts down dht and averager instances to avoid leaking processes

- handle py37-specific CancelledError correctly

hivemind/utils/threading.py

mryab · 2021-03-02T01:42:11Z

hivemind/client/averaging/__init__.py

-
+        background_fetcher = threading.Thread(daemon=True, target=_background_thread_fetch_current_state,
+                                              args=[self.pipe, weakref.WeakMethod(self.get_current_state)])
+        background_fetcher.start()  # note: this thread is daemon to avoid cyclic references


Move the comment before L128 (before actual initialization), «this thread» -> «we use a daemon to ...»

mryab · 2021-03-02T01:43:59Z

hivemind/client/averaging/__init__.py

+def _background_thread_fetch_current_state(pipe: mp.connection.Connection, get_current_state_weakref):
+    """ Executed in the host process as a background thread. Fetches the averager state when asked by peers. """


A type for get_current_state_weakref and an explanation of each argument in docstring would be really helpful

Co-authored-by: Max Ryabinin <[email protected]>

mryab

Version bump?

* copytree implementation for py37 compatibility (#162) * copytree implementation for py37 compatibility * Running tests for python3.7 * Increment version * Python3.7 notions * Remove pickle.loads in averager (#160) * Security update: remove pickle.loads in averager * add py37 to circleci config Co-authored-by: Alexander Borzunov <[email protected]> Co-authored-by: Max Ryabinin <[email protected]> * Support edge cases for DHT key/subkey/value, add tests, update .gitignore for pb2 (#167) * fix bug with subkey equals zero * add autogenerated protobuf files to .gitignore * test store and get "tricky" values in dht * Fix the remaining tests for py37 (#166) * DecentralizedAverager is now compatible with python37's acyncio exception * the problem was: grpc.aio with python37 raised concurrent.futures.CancelledError in some cases; * we relied on isinstance(asyncio.CancelledError, Exception) == False * but isinstance(concurrent.futures.CancelledError, Exception) == True * DecentralizedAverager now shuts down if dereferenced in the main process * though it won't shutdown if dereferenced in forks for obvious reasons * HIVEMIND_THREADS now actually works * test_averaging now shuts down dht and averager instances to avoid leaking processes Co-authored-by: Max Ryabinin <[email protected]> Co-authored-by: Max Ryabinin <[email protected]> * Move Averager metadata serialization out of user scope (#168) * move metadata serialization outside user scope * test_overcrowded: reduce the default number of peers * Handle edge cases in DecentralizedAverager (#171) * move metadata serialization outside user scope * retry averager.step on network errors * raise AllreduceException on partial tensor * test split/combine tensors, combine corrupted stream Co-authored-by: Max Ryabinin <[email protected]> * Fix a typo in quickstart.md (#174) * Serialize DHTID source with msgpack (#172) * Change DHTID serializer * Remove unused serializers * Add msgpack tuple serialization * Move CLI server launch script to hivemind/hivemind_cli (#173) * Cast environment variables to correct types * Compiling libp2p daemon on setup (#153) * add setup.py prototype * refactor * feat: add p2p daemon (#164) * Add p2p daemon * Test p2p daemon exits correctly * Impose restriction on elapsed time Co-authored-by: Ilya Kobelev <[email protected]> * compare golang versions using packaging.version * fix typo Co-authored-by: justheuristic <[email protected]> * move p2pd executable to hivemind/hivemind_cli Co-authored-by: Alexey Bukhtiyarov <[email protected]> Co-authored-by: justheuristic <[email protected]> Co-authored-by: Alexander Borzunov <[email protected]> Co-authored-by: Max Ryabinin <[email protected]> Co-authored-by: Michael Diskin <[email protected]> Co-authored-by: romakail <[email protected]> Co-authored-by: Ilya <[email protected]> Co-authored-by: Ilya Kobelev <[email protected]>

justheuristic added 2 commits March 2, 2021 00:50

[wip] separate test_training

f3a6008

rollback tests

57d1155

mryab requested review from mryab and leshanbog March 1, 2021 23:38

justheuristic added 2 commits March 2, 2021 03:13

- ensure that DecentralizedAverager will shutdown upon dereferencing

0417241

- handle py37-specific CancelledError correctly

kill stragglers in allreduce

5f7ad3b

justheuristic changed the title ~~Fix tests for py37~~ Fix the remaining tests for py37 Mar 2, 2021

justheuristic marked this pull request as ready for review March 2, 2021 00:29

justheuristic linked an issue Mar 2, 2021 that may be closed by this pull request

Optimize the tests #158

Closed

mryab requested changes Mar 2, 2021

View reviewed changes

justheuristic and others added 4 commits March 2, 2021 05:11

Update hivemind/utils/threading.py

9992bb2

Co-authored-by: Max Ryabinin <[email protected]>

address review comments by @mryab

620c5d5

Merge remote-tracking branch 'origin/py37-tests' into py37-tests

fb88b1f

address review comments by @mryab

9444ca8

justheuristic removed a link to an issue Mar 2, 2021

Optimize the tests #158

Closed

justheuristic requested a review from mryab March 2, 2021 13:54

mryab approved these changes Mar 2, 2021

View reviewed changes

leshanbog approved these changes Mar 2, 2021

View reviewed changes

justheuristic added 2 commits March 2, 2021 17:20

Merge branch 'master' into py37-tests

38fbfcd

bump version

3979134

justheuristic merged commit 690c9dc into master Mar 2, 2021

justheuristic deleted the py37-tests branch March 2, 2021 15:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the remaining tests for py37 #166

Fix the remaining tests for py37 #166

justheuristic commented Mar 1, 2021 •

edited

Loading

mryab Mar 2, 2021

mryab Mar 2, 2021

mryab left a comment

		def _background_thread_fetch_current_state(pipe: mp.connection.Connection, get_current_state_weakref):
		""" Executed in the host process as a background thread. Fetches the averager state when asked by peers. """

Fix the remaining tests for py37 #166

Fix the remaining tests for py37 #166

Conversation

justheuristic commented Mar 1, 2021 • edited Loading

mryab Mar 2, 2021

Choose a reason for hiding this comment

mryab Mar 2, 2021

Choose a reason for hiding this comment

mryab left a comment

Choose a reason for hiding this comment

justheuristic commented Mar 1, 2021 •

edited

Loading