Done.
1 Like
Sorry my fault for not reading carefully. Yes I guess they should.
except x_tfms is now a mismatch with this group. But it’s _tfms everywhere in the current code so it’s probably good.
Ugh. Hmm… I’m having trouble coming up with reasons why it shouldn’t be tfms_x.
I think it’s correct for it to be _tfms, it’s like _dl, _ds, etc. And in the current v1 code base it is _tfms everywhere. Moreover if you do decide to flip it, then what do we do here?
DataBunch(train_tds, valid_ds, bs=bs, train_tfms=batch_transforms, valid_tfms=batch_transforms)
how do we deal with valid_ and train_ being prefixes.
I just meant that it locally will break the flow of something_x, followed by x_something.
Perhaps it shouldn’t be ‘x’ in x_tfms, but something else and that will fix the issue, leaving _tfms as is?
Well the good news is that I removed that parameter today, so nothing to worry about now 
2 Likes
Let’s discuss the convention for log/exp/normal functions and their input variables and outputs:
In 001a_nn_basics.ipynb:
def log_softmax(x):
xe = x.exp()
return (xe / xe.sum(dim=-1).unsqueeze(1)).log()
def model(xb): return log_softmax(xb @ weights + bias)
preds = model(xb) # predictions
preds[0], preds.shape
Would it be better to replace preds with log_preds, since log_softmax returns a log()?
Right after that code we are dealing with negative log-likelihood. Would it be useful for def nll to name its log input vars as such? It’d just make the code reading so much more intuitive.
Note we have already started shifting towards this in v0’s fastai/metrics.py:
def recall(log_preds, targs, thresh=0.5, epsilon=1e-8):
preds = torch.exp(log_preds)
pred_pos = torch.max(preds > thresh, dim=1)[1]
tpos = torch.mul((targs.byte() == pred_pos.byte()), targs.byte())
return tpos.sum()/(targs.sum() + epsilon)
So here we know the expected input is log’ed and exp makes it “normal”.
The way the code written right now I can never tell whether the inputs are normal or log()ed/exp()ed.
Thoughts?
Not really… I think the problem really is that these aren’t actually predictions of anything, so preds is probably the wrong word. We could call it y_hat since that’s the normal name for an output in machine learning, or just output. Generally you assume the outputs are in the appropriate form to send to a loss function, as they are here.
I don’t think this particular naming discussion is easy or has any correct solution that works well everywhere. The fact that all the cross entropy functions in pytorch assume log-variables and our models will presumably always spit these out means that I think it’s better just to make sure these things are clearly documented.
Pytorch loss functions have standardize param naming that I think we should fit in with. I’ll fix nll to do that.
ok, thank you for sharing your take on it, Jeremy.
What about the metrics, e.g. the code I quoted above? would log_preds be a good convention for v1, or not? I suppose torch has a convention to follow there as well.
I don’t think pytorch really has any metrics AFAIK. I think it’s a useful naming convention to have for metrics.
Excellent. Let’s follow that then. Thank you, Jeremy.