Throw ValueError for wrong reduction in losses

[oke-aditya]

As discussed in #6457

The current problem

We do not restrict the reduction mode in losses. There are only three cases possible, ["none", "mean", "sum"]. If the user passes something like "xyz" we are not throwing any error or warning.

The solution

If users aren't passing one of the valid cases and using a random string instead. It's better to throw a ValueError suggesting them to use one of the correct choice.

We had thought of Enum based solution, but to keep better consistency with Pytorch core as discussed, we are adopting the above solution.

Edits required

Need to edit losses. which are namely
ciou_loss, diou_loss, giou_loss, and focal_loss

Also need to edit the relevant tests in test_ops.py

Additional Context

I think it's good first issue and some new contributor can take this 😃

Feel free to ping the maintainers or me if you need help. 😄

[NicolasHug]

Looking at https://github.com/pytorch/vision/pull/6457/files, it looks like this is taken care of in the original PR already? Or were you referring to other losses @oke-aditya ?

[datumbox]

I think @oke-aditya refers to all the other losses. I believe that's a good idea but if the ticket is intended for a new contributor, it's worth listing explicitly all the places were the change needs to happen and a potential idiom we want to adopt. This will help people who are not familiar with our code-base to pick it up.

[oke-aditya]

Yeah I'm referring to all the other losses. I will add further details to help new contributors 😃

[oke-aditya]

Well I hope I have edited the issue and explained it in bit more detail. I haven't given away the code of what needs to be done intentionally (as I believe some new contributor should try to figure out and interact with people while fixing)

cc @datumbox

[datumbox]

@oke-aditya Looks great to me! Thanks for helping us attract more contributors :)

[adityagandhamal]

I can work on this. Would you please assign me the task?

[oke-aditya]

@adityagandhamal
feel free to send PR and ping me as a reviewer. I can review.

Also btw how did you come to know about this issue?

[adityagandhamal]

I recently started using PyTorch and was browsing about, looking for anything to contribute to, and fortunately, I came upon this!

[datumbox]

@adityagandhamal Welcome! I've assigned the ticket to you, looking forward to your PR!

[adityagandhamal]

Yes. I've finished making the necessary changes to the code and am now working on the tests. Will send the PR soon.

[adityagandhamal]

While running a test on test_giou_loss specifically, I am running into an error AttributeError: module 'torchvision.ops' has no attribute 'generalized_box_iou_loss'

Quite strange to have encountered such an error given the loss function exists in the module

[datumbox]

This sounds weird. It seems like if TorchVision is not properly installed. I would start by a fresh conda environment and follow this guide. Let me know if you continue facing problems.

[adityagandhamal]

Yes, I've been following the same guide but nonetheless, I'll go for it again.
But before I give another run at this, I'd like you to know that I did come across an error after running python setup.py develop. It was regarding Microsoft Visual C++ Build Tools. It kept on returning the same error even after installing the necessary package.

And pardon me if this sounds naive but one has to clone the forked repo into their local machine but in the guide, why is it mentioned to clone the parent repo? git clone https://github.com/pytorch/vision.git

[oke-aditya]

I'd like you to know that I did come across an error after running python setup.py develop. It was regarding Microsoft Visual C++ Build Tools.

Can you send detailed log of the same? Sorry I can't help much as I use linux only.

but one has to clone the forked repo into their local machine but in the guide, why is it mentioned to clone the parent repo? git clone https://github.com/pytorch/vision.git

Yeah you need to clone the fork. The guide might be written to build from source and not to contribute.

[adityagandhamal]

Okay, after spending some time on StackOverflow and rebooting the system, now it seems everything is good to go.
Finished processing dependencies for torchvision==0.14.0a0+30b879f

@oke-aditya @datumbox
I'll make the edits, run the tests, and ping you if needed.
Thank you

[adityagandhamal]

Just to test whether torchvision is installed properly, I tried importing it only to get a windows popup error

I can't figure this out

[oke-aditya]

Actually you are doing quite a difficult thing. Building on windows. Much harder. WSL2 can help you though to make it easier.

Have you run

python setup.py develop ?

[oke-aditya]

Also I believe we should have a step by step guide for windows @datumbox most of us don't probably use windows and it's a pain point for new developers to setup.

For Linux it's very very straightforward and hardly takes an hour.

[adityagandhamal]

Have you run

python setup.py develop ?

Yes, I have. It ran without giving any error and returned Finished processing dependencies for torchvision==0.14.0a0+30b879f

[adityagandhamal]

Yes, I agree that developing on WSL is always a better option.

On WSL as well, should the same steps be followed (according to the guide) as before?

[oke-aditya]

Yeah WSL you can just install Ubuntu and follow the steps mentioned for Linux.
It will have gcc compiler and hence your half job will be done.

On WSL I believe you will need

1. Install miniconda
2. Create env and install Python
3. Install pytorch nightly. For now cpuonly would suffice.
4. clone torchvision. Follow the contribution guide

Basically 4 is

conda install libpng jpeg
python setup.py develop
pip install flake8 typing mypy pytest pytest-mock scipy

[adityagandhamal]

WSL2 is great @oke-aditya! torchvision has successfully set up. No absurd errors so far.
Thank you

[oke-aditya]

One of the nice things about good first issues is that it helps to understand what the difficulty a new contributor may face.

It's quite ok and in fact expected that a new contributor will need help in getting started and setting up. So nothing wrong about seeking help.

Keep going @adityagandhamal (btw our first name is the same 😃)

[adityagandhamal]

@oke-aditya @datumbox
I have sent the PR, open to suggestions