Skip to content

Conversation

@guptaaryan16
Copy link
Contributor

@guptaaryan16 guptaaryan16 commented Feb 4, 2023

Fixes #2837

Description: I have added the function for model_transform

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@github-actions github-actions bot added the module: engine Engine module label Feb 4, 2023
@guptaaryan16 guptaaryan16 changed the title Add model_transform in create supervised trainer Add model_transform in create supervised trainer[WIP] Feb 4, 2023
Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@guptaaryan16 thanks for the PR ! Looks good to me. Just few minor improvements and please add a test into https://github.com/pytorch/ignite/blob/master/tests/ignite/engine/test_create_supervised.py

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 4, 2023

cc @david-waterworth if case if you would like to review this PR

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 8, 2023

@guptaaryan16 thanks for the update but let's improve test code

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 14, 2023

@guptaaryan16 can you please update this PR to make it merged

@guptaaryan16
Copy link
Contributor Author

Hey @vfdev-5 , sure I am making the changes by today or tomorrow EOD and I apologise for the delay in the same

@vfdev-5 vfdev-5 changed the title Add model_transform in create supervised trainer[WIP] Add model_transform in create supervised trainer Feb 14, 2023
@guptaaryan16
Copy link
Contributor Author

Hey @vfdev-5 can you give me an idea on how to update the API doc

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 17, 2023

@guptaaryan16 I pushed this PR : guptaaryan16#1 to your master branch on how to update the tests. Could you please merge this PR quickly to unblock this PR as we are planning the release. Thanks

@guptaaryan16
Copy link
Contributor Author

guptaaryan16 commented Feb 17, 2023

Okay I am working in this issue and hope to complete this today. Thanks for helping with the tests should I update the docs of the API

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 17, 2023

Okay I am working in this issue and hope to complete this today. Thanks for helping with the tests should I update the docs of the API

Thanks @guptaaryan16 ! Can you add versionchanged into the docstrings and also usage example can be helpful

.. versionchanged:: 0.4.7
Added Gradient Accumulation.

    .. versionchanged:: 0.4.7
        Added `model_transform` to transform model's output

@guptaaryan16
Copy link
Contributor Author

@vfdev-5 I don't know why but to make the tests work I had to remove the bias variable in the tests as it gives a NoneType error

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 17, 2023

@vfdev-5 I don't know why but to make the tests work I had to remove the bias variable in the tests as it gives a NoneType error

I checked the tests locally and they were passing. Let's revert this change and see the exact error message

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 17, 2023

OK, I see, we have to remove all related lines:

model.fc.bias.data.zero_()

assert model.fc.bias.item() == approx(0.0)

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @guptaaryan16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: engine Engine module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Model Ouput Transform

2 participants