Abbreviations for train/validation/test variables

fastai fastai dev
50

I have to apologize for starting this. You’ve definitely covered this and I think I’m one of those programmers that you’re referring to here in the quote below. I’m going to try to make a conscious effort to not let knee-jerk reactions on short names that I’ve developed over the years get in the way of understanding the benefits. (I’ll try!) :grinning: Anyway- it’s definitely a new approach to me and I know it’s going to be for many others.

Everyone has strong opinions about coding style, except perhaps some very experienced coders, who have used many languages, who realize there’s lots of different perfectly acceptable approaches. The python community has particularly strongly held views, on the whole. I suspect this is related to Python being a language targeted at beginners, and therefore there are a lot of users with limited experience in other languages; however this is just a guess.

I think it’ll be helpful to link to what you’re referring to- yes, we’re beating a dead horse.

Forum discussion

Style guide

3 Likes
51

Thanks, good reads.

Makes more sense now why you would want more condensed code for math related stuff, especially when contributing code as opposed to just using the library.

1 Like
52

@stas FYI @rachel has just started adding prose to the first notebook. To avoid annoying problems with merging, you probably want to know about this:

53

Yes, nbdime is great. Thank you.

Perhaps @rachel could notify me when her changes have been merged - so that we don’t step on each other’s toes, and I will resume then.

(I’ve been migrating to kubuntu 18.04 so I’ve been away)

54

Actually the toe-stepping may be a problem, since @313V has started working on adding prose to the other notebooks now too. So I’ve suggested to him that maybe it’s easier for him to do the renaming as he goes. If you’ve already made some changes @stas it might be a good idea to push them.

55

No, not yet. I was just finishing the setup of the new 18.04 env.

Actually the toe-stepping may be a problem, since @313V has started working on adding prose to the other notebooks now too.

ok, then I will occupy myself with other things for now.

and there are 2 outstanding questions waiting for your attention @jeremy (naming-wise):

Thanks

56

Python modules use _, so let’s do that in nbs too.

I don’t think so - because this pattern is nearly always in for xb,yb in dl, where we don’t want long names. I know in the notebooks we have this pattern separated out, but I’d like to keep it consistent with how people will generally see it in the code.

57

@313V please let me know if I can help you with adding prose to the notebooks. I have been following the discussions closely and have free time available.

58

help would be great, technical writing isn’t necessarily my strength anyway so at minimum someone to help review and edit would be awesome and i’m sure you could add a lot more than that.
What time zone are you in? Are you on any kind of video chat like skype/gchat/facetime etc? It would be cool to chat face to face and form a plan of attack.

1 Like
59

Sorry, this got miscommunicated, quoting again:

loss_fn(model(x_valid[0:bs]), y_valid[0:bs])
xb, yb = next(iter(valid_dl))

Should x_valid and y_valid be valid_x and valid_y? I quoted the second line because of valid_dl

More examples of inconsistencies from the current codebase:

data = DataBunch            (train_ds, valid_ds,
data = DataBunch.from_arrays(x_train,y_train,x_valid,y_valid, x_tfms=mnist2image)\n",

should these be: train_x,train_y,valid_x,valid_y? It’d now look:

data = DataBunch            (train_ds, valid_ds,
data = DataBunch.from_arrays(train_x,train_y,valid_x,valid_y, x_tfms=mnist2image)\n",

except x_tfms is now a mismatch with this group. But it’s _tfms everywhere in the current code so it’s probably good.

60

Good, so could one of you with commit access do the filename renaming - it’d complicate things to do this via PR, if changes happen in the files meanwhile.

and also I mentioned keeping consistent casing would be awesome too - it seems that all-low-case are the majority of files so far, so perhaps renaming files to lowercase would be a sensible choice.

if you have https://stackoverflow.org/wiki/Rename.pl it’s 2 secs:

rename.pl 's|-|_|g' *ipynb
rename.pl '$_=lc $_' *ipynb

Thank you.

61

Done.

1 Like
62

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.

63

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?

64

Well the good news is that I removed that parameter today, so nothing to worry about now :wink:

2 Likes
65

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?

66

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.

67

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.

68

I don’t think pytorch really has any metrics AFAIK. I think it’s a useful naming convention to have for metrics.

69

Excellent. Let’s follow that then. Thank you, Jeremy.