[GitHub] eric-haibin-lin commented on a change in pull request #10292: [MXNET-243] Allow custom merger in KVStore
eric-haibin-lin commented on a change in pull request #10292: [MXNET-243] Allow
custom merger in KVStore
URL: https://github.com/apache/incubator-mxnet/pull/10292#discussion_r180845317
##
File path: include/mxnet/kvstore.h
##
@@ -243,6 +243,29 @@ class KVStore {
str_updater_ = updater;
}
+ struct MergeMeta {
Review comment:
Would these meta information be sufficient for any kind of custom merger?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] eric-haibin-lin commented on a change in pull request #10292: [MXNET-243] Allow custom merger in KVStore
eric-haibin-lin commented on a change in pull request #10292: [MXNET-243] Allow
custom merger in KVStore
URL: https://github.com/apache/incubator-mxnet/pull/10292#discussion_r180845317
##
File path: include/mxnet/kvstore.h
##
@@ -243,6 +243,29 @@ class KVStore {
str_updater_ = updater;
}
+ struct MergeMeta {
Review comment:
Would these information be sufficient for any kind of custom merger?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] eric-haibin-lin commented on a change in pull request #10292: [MXNET-243] Allow custom merger in KVStore
eric-haibin-lin commented on a change in pull request #10292: [MXNET-243] Allow
custom merger in KVStore
URL: https://github.com/apache/incubator-mxnet/pull/10292#discussion_r180844654
##
File path: tests/nightly/dist_sync_kvstore.py
##
@@ -279,10 +279,11 @@ def check_compr_random(kv, threshold, nworker):
assert_almost_equal(diff.asnumpy(), decompr)
print ('worker '+str(my_rank)+' started with non compression tests')
-check_default_keys(kv, my_rank, nworker)
check_row_sparse_keys(kv, my_rank, nworker)
check_row_sparse_keys_with_zeros(kv, my_rank, nworker)
check_big_row_sparse_keys(kv, my_rank, nworker)
+kv.set_merger(mx.merger.create('accumulate'))
Review comment:
Is this only for dist kvstore? Can we also have unit test for "device"
kvstore in
https://github.com/apache/incubator-mxnet/blob/master/tests/python/gpu/test_kvstore_gpu.py
and for "local" kvstore in
https://github.com/apache/incubator-mxnet/tree/master/tests/python/unittest/test_kvstore.py
?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] eric-haibin-lin commented on a change in pull request #10292: [MXNET-243] Allow custom merger in KVStore
eric-haibin-lin commented on a change in pull request #10292: [MXNET-243] Allow
custom merger in KVStore
URL: https://github.com/apache/incubator-mxnet/pull/10292#discussion_r180639488
##
File path: python/mxnet/kvstore.py
##
@@ -583,6 +589,48 @@ def _set_updater(self, updater):
check_call(_LIB.MXKVStoreSetUpdaterEx(self.handle, self._updater_func,
self._str_updater_func, None))
+def _send_agg_to_servers(self, fn_name, agg_fn):
+is_worker = ctypes.c_int()
+check_call(_LIB.MXKVStoreIsWorkerNode(ctypes.byref(is_worker)))
+# pylint: disable=invalid-name
+if 'dist' not in self.type or not is_worker.value: # pylint:
disable=unsupported-membership-test
+return False
+try:
+# use ASCII protocol 0, might be slower, but not a big ideal
+ser_agg_fn = py_str(pickle.dumps({fn_name: agg_fn}, 0))
+except:
+raise
+self._send_command_to_servers(0, ser_agg_fn)
+return True
+
+def set_merger(self, merger):
+if not self._send_agg_to_servers('merger', merger):
+self._set_merger(merger)
+
+def _set_merger(self, merger):
+"""Sets a push updater into the store.
Review comment:
Could you move the doc string to the public interface "set_merger"?
Otherwise it won't be included in API webpage.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] eric-haibin-lin commented on a change in pull request #10292: [MXNET-243] Allow custom merger in KVStore
eric-haibin-lin commented on a change in pull request #10292: [MXNET-243] Allow
custom merger in KVStore
URL: https://github.com/apache/incubator-mxnet/pull/10292#discussion_r180639323
##
File path: python/mxnet/kvstore.py
##
@@ -83,6 +83,22 @@ def updater_handle(key, lhs_handle, rhs_handle, _):
updater(key, lhs, rhs)
return updater_handle
+class MergeMeta(ctypes.Structure):
+_fields_ = [('num_merged', ctypes.c_size_t),
+('sender_id', ctypes.c_int),
+('timestamp', ctypes.c_int)]
+MergeMetaHandle = ctypes.c_void_p
Review comment:
Can we put this in base.py?
https://github.com/apache/incubator-mxnet/blob/master/python/mxnet/base.py#L120
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
