MultiNodeChainList with self branching #102

levelfour · 2017-08-06T01:16:13Z

In #98 , MultiNodeChainList does not allow to send / recv to the same process (see https://github.com/chainer/chainermn/blob/master/chainermn/link.py#L129).
This pull request extends MultiNodeChainList a little bit to allow it.
See examples in test_link.py:

class BranchParent2(chainermn.MultiNodeChainList):
    def __init__(self, size, comm, rank_children):
        super(BranchParent2, self).__init__(comm=comm)
        ranks = [comm.rank] + rank_children
        self.add_link(BranchSubA(size), rank_in=None, rank_out=ranks)
        self.add_link(BranchSubA(size), rank_in=comm.rank, rank_out=comm.rank)
        self.add_link(BranchSubB(size), rank_in=ranks, rank_out=None)


class BranchParent3(chainermn.MultiNodeChainList):
    def __init__(self, size, comm, rank_children):
        super(BranchParent3, self).__init__(comm=comm)
        ranks = rank_children + [comm.rank]
        self.add_link(BranchSubA(size), rank_in=None, rank_out=ranks)
        self.add_link(BranchSubA(size), rank_in=comm.rank, rank_out=comm.rank)
        self.add_link(BranchSubB(size), rank_in=ranks, rank_out=None)


class BranchParent4(chainermn.MultiNodeChainList):
    def __init__(self, size, comm, rank_children):
        super(BranchParent4, self).__init__(comm=comm)
        ranks = rank_children + [comm.rank]
        ranks = ranks[1:] + ranks[0:1]
        self.add_link(BranchSubA(size), rank_in=None, rank_out=ranks)
        self.add_link(BranchSubA(size), rank_in=comm.rank, rank_out=comm.rank)
        self.add_link(BranchSubB(size), rank_in=ranks, rank_out=None)

In these examples, we can specify comm.rank (the rank on which the models would be instantiated) for rank_in or rank_out.

keisukefukuda · 2017-08-09T04:53:03Z

chainermn/functions/point_to_point_communication.py

@@ -104,6 +110,7 @@ def send(x, communicator, rank, tag=0):

    """
    chainer.utils.experimental('chainermn.functions.send')
+    assert rank != communicator.rank


Maybe we can add an assertion comment like "Cannot send to the local process itself" or something.

(Or should it be an internal error and should not happen?)

Fixed (use ValueError instead).

keisukefukuda · 2017-08-09T04:57:06Z

chainermn/functions/point_to_point_communication.py

@@ -136,6 +143,7 @@ def recv(communicator, rank, delegate_variable=None, tag=0, device=-1):

    """
    chainer.utils.experimental('chainermn.functions.recv')
+    assert rank != communicator.rank


assertion comment

keisukefukuda · 2017-08-09T05:15:09Z

chainermn/link.py

-                    # the same edge more than twice.
-                    delegate_variable = None
+                # Prevent "double-backwarding," i.e., backprop
+                # the same edge more than twice.


More than once ?
(only == 1 is good && >= 2 is not good, right?)

Exactly. Don't we say "more than twice" in this case?

In case of ratio, there's no big difference between "> 2x" and ">= 2x" , so both of them can be translated into a Japanese word "2倍以上". But in this case, I don't think it applies.
"... should be called exactly once" would be less confusing.

Thank you for suggestion. Fixed it.

iwiwi · 2017-08-09T09:45:21Z

chainermn/link.py


        self._rank_inouts.append((rank_in, rank_out))

    def __call__(self, *inputs):
+        comm_queue = queue.Queue()


How about checking comm_queue is empty at the end of this function?

Thank you. I fixed it.

iwiwi · 2017-08-10T02:03:13Z

chainermn/link.py

@@ -221,6 +221,8 @@ def __call__(self, *inputs):
                            x, self._comm,
                            rank=_rank_out)

+        assert comm_queue.empty()


I think this happens with user errors, so exceptions should be used

iwiwi · 2017-08-10T02:31:42Z

LGTM

levelfour added 7 commits August 6, 2017 02:12

Self branching send / recv.

9ae7203

Merge branch 'master' into self-branch

08c98b8

Add the other branching test.

a2a9088

Add another branching test (randomized order of send/recv).

30fc321

Fix import.

9b303ae

Do not hardcode rank.

2e198b1

Add a test (twisting model).

7923111

keisukefukuda reviewed Aug 9, 2017

View reviewed changes

levelfour added 4 commits August 9, 2017 14:55

Use ValueError.

81b336b

Fix a comment.

05d2632

Check rank.

77fa9e6

Forget a blacket.

6ddbc51

iwiwi reviewed Aug 9, 2017

View reviewed changes

Make sure queue is empty.

307d6c2

iwiwi reviewed Aug 10, 2017

View reviewed changes

Use ValueError instead.

aafafe8

iwiwi merged commit 98595c2 into chainer:master Aug 10, 2017

levelfour deleted the self-branch branch August 11, 2017 15:34

iwiwi added this to the v1.0.0 milestone Aug 31, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MultiNodeChainList with self branching #102

MultiNodeChainList with self branching #102

levelfour commented Aug 6, 2017 •

edited

Loading

keisukefukuda Aug 9, 2017

keisukefukuda Aug 9, 2017

levelfour Aug 9, 2017

keisukefukuda Aug 9, 2017

keisukefukuda Aug 9, 2017

keisukefukuda Aug 9, 2017

levelfour Aug 9, 2017

keisukefukuda Aug 9, 2017 •

edited

Loading

levelfour Aug 9, 2017

keisukefukuda Aug 9, 2017

iwiwi Aug 9, 2017

levelfour Aug 10, 2017

iwiwi Aug 10, 2017

iwiwi commented Aug 10, 2017

MultiNodeChainList with self branching #102

MultiNodeChainList with self branching #102

Conversation

levelfour commented Aug 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keisukefukuda Aug 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iwiwi commented Aug 10, 2017

levelfour commented Aug 6, 2017 •

edited

Loading

keisukefukuda Aug 9, 2017 •

edited

Loading