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

Implement assign function #244

Merged
merged 19 commits into from Apr 23, 2019

Conversation

Projects
None yet
3 participants
@takuseno
Copy link
Contributor

commented Sep 12, 2018

Hi, @TE-TakuyaNarihira san.

I added F.assign function that behaves like tf.assign.

This is used for manual assignment or manual variable update.

usecase1: manual assignment

x = nn.Variable((3, 3))
y = nn.Variable.from_numpy_array(np.random.random((3, 3)))
assign_op = F.assign(y, x)

x.d = np.random.random((3, 3))
assign_op.forward()
print((x.d == y.d).all()) # True
print((assign_op.d == x.d).all()) # True

usecase2: manual update

lr = 1.0
original = np.random.random((3, 3))

grad = F.constant(1.0, (3, 3))
y = nn.Variable.from_numpy_array(original)
train_op = F.assign(y, y + lr * grad)

train_op.forward()
print((y.d == (original + 1.0)).all()) # True, or False because of rounding floating points

F.assign will be useful to implement syncing multiple parameters or manual SGD implementation in static graph way.
In backwarding, F.assign propagates gradients to destination Variable.

TODO

  • add unit testing
  • backward implementation
@takuseno

This comment has been minimized.

Copy link
Contributor Author

commented Sep 14, 2018

Hi, @TE-TakuyaNarihira san.

Output of tf.assign does not propagate gradients to the original variable. Then F.assign does same bahavior for now. I'm concerning about the order of execution, which is dealt by control_dependencies in tensorflow. But I think we can add this function casually because there is noway to assign values to certain variables in static way right now.

@takuseno takuseno changed the title [WIP] Implement assign function Implement assign function Sep 14, 2018

@takuseno

This comment has been minimized.

Copy link
Contributor Author

commented Jan 10, 2019

@TE-TakuyaNarihira , @TE-AkioHayakawa , @TE-KazukiYoshiyama
Hi! I've modified implementation to use copy_from, which would work both on CPU and GPU without additional cuda codes. And I've confirmed my unit test passed on my machine.

@takuseno

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

@TE-AkioHayakawa backward done.

Show resolved Hide resolved src/nbla/function/generic/assign.cpp Outdated
Show resolved Hide resolved src/nbla/function/generic/assign.cpp Outdated
@TE-AkioHayakawa

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2019

@takuseno
Thanks! I've reviewed your PR.
Please reflect my reviews if you agree to my review.

And now I also run CI test for this branch.

@takuseno

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

@TE-AkioHayakawa
Hi! Thank you for reviewing my codes! I've updated my implementation. Please check them out!

@TE-TakuyaNarihira
Copy link
Contributor

left a comment

Thanks for your contribution @takuseno. I've also commented on a few things. Please address these.

Show resolved Hide resolved src/nbla/function/generic/assign.cpp Outdated
Show resolved Hide resolved src/nbla/function/generic/assign.cpp Outdated
@takuseno

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

@TE-TakuyaNarihira
Hi! I think my latest implementation meets your expectation. I'll do the same refactorings at batch_inv and batch_det functions.

@TE-TakuyaNarihira

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2019

Looks perfect!
Please wait for internal CI testing results.

@TE-AkioHayakawa

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2019

@takuseno
Internal CI testing has passed! And I think your codes look good!
Finally, could you do auto-format?

If you forget how to do auto-format, let me know.

@takuseno

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2019

@TE-AkioHayakawa
I'll do it later! Thank you!

@takuseno

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2019

@TE-AkioHayakawa TE-AkioHayakawa merged commit 29f7feb into sony:master Apr 23, 2019

@TE-AkioHayakawa

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2019

@takuseno
Marged!
Thanks again for your continuous contribution!

@takuseno takuseno deleted the takuseno:implement_assign_function branch Apr 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.