chainer / chainercv

ChainerCV: a Library for Deep Learning in Computer Vision
MIT License
1.48k stars 304 forks source link

Change links to override forward #835

Closed crcrpar closed 5 years ago

knorth55 commented 5 years ago

From which version can we use forward function?

crcrpar commented 5 years ago

@knorth55 V5. Here is the reference: https://docs.chainer.org/en/stable/upgrade.html?highlight=upgrade#use-forward-instead-of-call-in-links.

yuyu2172 commented 5 years ago

pfnCI, test this please

pfn-ci-bot commented 5 years ago

Successfully created a job for commit f7b1368:

yuyu2172 commented 5 years ago

Please fix these errors.


2019-03-18 11:10:02.161407 STDOUT 2074] | def _predict(self, imgs):
-- | --
2019-03-18 11:10:02.161414 STDOUT 2074] | xs = chainer.Variable(self.xp.asarray(imgs))
2019-03-18 11:10:02.161421 STDOUT 2074] | with chainer.using_config('train', False):
2019-03-18 11:10:02.161427 STDOUT 2074] | >           scores = F.softmax(self.forward(xs)).array
2019-03-18 11:10:02.161434 STDOUT 2074] | E           AttributeError: Parameterized test failed.
2019-03-18 11:10:02.161440 STDOUT 2074] | E
2019-03-18 11:10:02.161448 STDOUT 2074] | E           Base test method: TestPSPNetResNet.test_predict_gpu
2019-03-18 11:10:02.161455 STDOUT 2074] | E           Test parameters:
2019-03-18 11:10:02.161486 STDOUT 2074] | E             model: <class 'chainercv.experimental.links.model.pspnet.pspnet.PSPNetResNet50'>
2019-03-18 11:10:02.161524 STDOUT 2074] | E
2019-03-18 11:10:02.161533 STDOUT 2074] | E
2019-03-18 11:10:02.161539 STDOUT 2074] | E           (caused by)
2019-03-18 11:10:02.161546 STDOUT 2074] | E           AttributeError: 'PSPNetResNet101' object has no attribute 'forward'
2019-03-18 11:10:02.494948 STDOUT 2074] | self = <chainercv.experimental.links.model.pspnet.pspnet.PSPNetResNet50 object at 0x7f88b82a05d0>
-- | --
2019-03-18 11:10:02.494955 STDOUT 2074] | imgs = array([[[[   0.       ,    0.       ,    0.       , ...,    0.       ,
2019-03-18 11:10:02.494962 STDOUT 2074] | ...  , ...,   39.060997 ,
2019-03-18 11:10:02.494969 STDOUT 2074] | 128.061    ,  -56.939003 ]]]], dtype=float32)
2019-03-18 11:10:02.494975 STDOUT 2074] |  
2019-03-18 11:10:02.494976 STDOUT 2074] | def _predict(self, imgs):
2019-03-18 11:10:02.494982 STDOUT 2074] | xs = chainer.Variable(self.xp.asarray(imgs))
2019-03-18 11:10:02.494988 STDOUT 2074] | with chainer.using_config('train', False):
2019-03-18 11:10:02.494994 STDOUT 2074] | >           scores = F.softmax(self.forward(xs)).array
2019-03-18 11:10:02.495000 STDOUT 2074] | E           AttributeError: Parameterized test failed.
2019-03-18 11:10:02.495006 STDOUT 2074] | E
2019-03-18 11:10:02.495012 STDOUT 2074] | E           Base test method: TestPSPNetResNet.test_predict_gpu
2019-03-18 11:10:02.495018 STDOUT 2074] | E           Test parameters:
2019-03-18 11:10:02.734602 STDOUT 2074] | E             model: <class 'chainercv.experimental.links.model.pspnet.pspnet.PSPNetResNet50'>
2019-03-18 11:10:02.734638 STDOUT 2074] | E
2019-03-18 11:10:02.734648 STDOUT 2074] | E
2019-03-18 11:10:02.734656 STDOUT 2074] | E           (caused by)
2019-03-18 11:10:02.734663 STDOUT 2074] | E           AttributeError: 'PSPNetResNet50' object has no attribute 'forward'
crcrpar commented 5 years ago

I apologize to have missed chainercv/experimental/links and I'm working on them.

knorth55 commented 5 years ago

can you also modify FCIS and Yolov2Tiny in experimental if possible?

yuyu2172 commented 5 years ago

Sorry. I had not checked this seriously. Could you change chainercv.utils.testing.constant_stub_link.py? Also, could you change all TrainChains in examples?

yuyu2172 commented 5 years ago

pfnCI, test this please

pfn-ci-bot commented 5 years ago

Successfully created a job for commit 45624ca:

Hakuyume commented 5 years ago

How do you think of the following items that contain __call__? *.rst files https://github.com/chainer/chainercv/blob/45624ca3badfb3d530a9726375fa4ff9df7c5ced/chainercv/utils/testing/constant_stub_link.py#L14-L15 https://github.com/chainer/chainercv/blame/45624ca3badfb3d530a9726375fa4ff9df7c5ced/examples/faster_rcnn/README.md#L16

crcrpar commented 5 years ago

I didn't change them because chainer.Link and chainer.Chain didn't include :meth:forward in their documentation.

However, I noticed that other Links like L.Convolution2D and Chains like ResNet50Layers included :meth:forward.

So, let me change them too.

Hakuyume commented 5 years ago

*.rst files still contain __call__. https://github.com/chainer/chainercv/blame/7af64e81012759314f7cff467bc19b350b7bcab7/docs/source/reference/links/faster_rcnn.rst#L26

crcrpar commented 5 years ago

Do you think it's better to remove all __call__?

2019年3月22日(金) 13:26 Toru Ogawa notifications@github.com:

*.rst files still contain call.

https://github.com/chainer/chainercv/blame/7af64e81012759314f7cff467bc19b350b7bcab7/docs/source/reference/links/faster_rcnn.rst#L26

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/chainer/chainercv/pull/835#issuecomment-475489822, or mute the thread https://github.com/notifications/unsubscribe-auth/APcP051MJ7rYosDtXfH2AqxVBCdvkxPAks5vZFtwgaJpZM4b4Jdy .

Hakuyume commented 5 years ago

Since __call__ does not have its docstring and nothing is rendered, we should remove them.

yuyu2172 commented 5 years ago

LGTM