Pytorch-lightning: Make all abbreviations: (nb --> num) and (i --> idx)

[elliotwaite]

Is your feature request related to a problem? Please describe.
The code currently uses two different abbreviations for number (nb and num) and index (idx and i).

An example of nb:
def training_step(self, batch, batch_nb):
https://williamfalcon.github.io/pytorch-lightning/LightningModule/RequiredTrainerInterface/#training_step

An example of num:
def log_metrics(self, metrics, step_num):
https://williamfalcon.github.io/pytorch-lightning/Trainer/Logging/#custom-logger

An example of idx:
def training_step(self, batch, batch_nb, optimizer_idx):
https://williamfalcon.github.io/pytorch-lightning/LightningModule/RequiredTrainerInterface/#training_step

An example of i:
def optimizer_step(self, current_epoch, batch_nb, optimizer, optimizer_i, second_order_closure=None):
https://williamfalcon.github.io/pytorch-lightning/Trainer/hooks/#optimizer_step

Describe the solution you'd like
I think switching to using consistent abbreviations for both number and index would improve the design quality of the API.

Although nb is currently used more than num, PyTorch uses num ubiquitously (e.g. torch.nn.RNN(num_layers=...)), so using num would probably make the API feel more familiar to users, and is the abbreviation I would suggest. I'm less sure which is better between idx and i, but it looks like there are fewer current uses of i, so changing those to use idx would probably be easier.

I know one of Lightning's design principles is a backward-compatible API, but there are some options for updating argument names with deprecation warnings that could be considered:
https://stackoverflow.com/questions/49802412/how-to-implement-deprecation-in-python-with-argument-alias

Also if this change is ever going to be made, the sooner probably the better, while Lightning is still in its relatively early stages of development.

Describe alternatives you've considered
Alternatively, if this would be too much of a breaking change, perhaps this change could be delayed until when a future breaking change is released.

I'm interested to hear others' opinions on this.

[Borda]

I guess/propose to rename all ..._i to ..._idx and also it makes sense to unify the nb and num
@williamFalcon @Ir1d @jeffling do you have a preference about nb/mun e.g. sklearn is mostly using num but I personally prefer nb

[jeffling]

personally, prefer 'num'. Anecdoctally, I've worked on teams that have used both and I've overheard more new folks ask about nb over num :)

More importantly: The point that pytorch also uses num is very convincing, seeing as Lightning should feel as familiar as possible to any researcher coming into the codebase. I think that hits the design principles.

For the Trainer arguments, it's a bit trickier so we might want to have some backwards compatibility like OP stated.

[williamFalcon]

Happy to standardize around num if pytorch uses it.
All the i should become idx.

Anyone want to take this as a PR? (but we need to keep backwards compatibility)

[Borda]

@williamFalcon do you want to hold back compatibly for how long? for several next releases and then clean the API or forever (this will make the API quite messy in the long term)?
I would make it like

warnings.warn("`root_module` module has been renamed to `lightning` since v0.5.3"
              " and will be removed in v0.8.0", DeprecationWarning)

[elliotwaite]

@williamFalcon @Borda Sorry for the delay in reply, I was subscribed to too many GitHub issues. Those changes look good to me! I also like the switch from batch_num to batch_idx. The only variables that now stand out to me when looking over the PR are:

step_idx, epoch_idx, max_num_epochs, andmin_num_epochs

which I think would be more natural if simplified to:

step, epoch, max_epochs, and min_epochs

This, however, is a separate issue than the choice of abbreviation, so perhaps this should be discussed in a separate thread if it's going to be discussed at length. But since those variables are being updated, perhaps addressing these at the same time would be a good idea if any of them are going to be changed. Any thoughts on these potential simplifications?

[elliotwaite]

I opened a new thread to discuss the above potential variable simplifications: https://github.com/williamFalcon/pytorch-lightning/issues/584

[Borda]

I would say the the meaning is clean now but I like this proposal... =)

[williamFalcon]

i vote for making these changes now as well