Add size assert, refactor #3
Conversation
|
@karpathy similarly to the circular buffer logic PR from ngram repo (see EurekaLabsAI/ngram#10) - happy to modify Having said that it's a tradeoff between simplicity and robustness. Lmk if you'd like to see that change. |
tensor1d.c
Outdated
| if (start > t->size) { start = t->size; } | ||
| if (end > t->size) { end = t->size; } | ||
| // 2) handle out-of-bounds indices: clip to [0, t->size] range | ||
| start = fmin(fmax(start, 0), t->size); |
There was a problem hiding this comment.
The fmin and fmax in C are on double, so there will be a weird cast from int to double here, would prefer to not do that. An alternative to the ifs would possibly be a ternary expression, I'd be ok with that
There was a problem hiding this comment.
@karpathy ok good point, my bad here -> fixed, see whether you like this solution
tensor1d.c
Outdated
| @@ -41,6 +41,7 @@ inline int ceil_div(int a, int b) { | |||
| // similar to torch.Storage | |||
|
|
|||
| Storage* storage_new(int size) { | |||
| assert(size > 0); | |||
| @@ -30,7 +30,7 @@ void *malloc_check(size_t size, const char *file, int line) { | |||
| // ---------------------------------------------------------------------------- | |||
| // utils | |||
|
|
|||
| inline int ceil_div(int a, int b) { | |||
There was a problem hiding this comment.
agree on getting rid of inline if it causes issues. I didn't realize inline was contentious and has sharp edges.
Minor refactor + added size assert.
inlinewas causing compile failures on my Linux machine, i think if we want to make it inline we'd have to move it to the header file (but then again see my next comment on simplicity vs robustness tradeoff so if we err on the simplicity side i say let's just omit this minor optimization (?)).