llm.c icon indicating copy to clipboard operation
llm.c copied to clipboard

a little reorg :-p

Open zocterminal opened this issue 1 year ago • 5 comments

Reorganized source code to consolidate copied functions and structures:

  • common stuff is now in train_common.c and train_common.h
  • cuda version needs to define TRAIN_CUDA before including the header or the .c file.
  • using the checkFopen(), mallocCheck() etc. functions in train_gpt2.c also.

It may be possible to consolidate the reading of the model a bit more by using ifdefs with the TRAIN_CUDA and TRAIN_CPU macros.

Enjoy.

zocterminal avatar Apr 18 '24 14:04 zocterminal

So I want to do this, I'm just not sure when :)

karpathy avatar Apr 18 '24 15:04 karpathy

I certainly don't want to go down the IFDEF switching hell.

karpathy avatar Apr 18 '24 15:04 karpathy

Yes, it would be great if we can reduce / eliminate the use of #ifdef's in the source.

rosslwheeler avatar Apr 18 '24 15:04 rosslwheeler

What's hell for some is just a cozy summer vacation to others ;-)

zocterminal avatar Apr 18 '24 15:04 zocterminal

Having most of a file covered in ifdefs sure messes with the elegant simplicity. Compiler quirks should be few, so its the targets that are the real concern, isn't it? But it is C after all, maybe the pragmatic approach would be to refactor/split things out to separate files if and when a 4th platform is added? 2 seems managable, 3 might even be. Four could become too much to keep track of. I am here to learn though.

dagelf avatar Apr 18 '24 18:04 dagelf

Closing (has hard to fix merge conflicts now, new version with different approach is https://github.com/karpathy/llm.c/pull/177).

zocterminal avatar Apr 19 '24 09:04 zocterminal