minGPT icon indicating copy to clipboard operation
minGPT copied to clipboard

Refactor for modern 2022 python style and usage

Open mattsta opened this issue 2 years ago • 3 comments

No actual functionality changes, just a lot of cleanup to make the project look like it belongs in the present instead of something half abandoned from 10 years ago.

Some of these changes are important because other projects copy this project over and over again, so every legacy code architecture pattern here gets proliferated unnecessarily into the future for all time.

There's simple solutions to hacks I've seen in this codebase copied and pasted into dozens of other projects (like the 10 line "continuous iterator" hack which actually has a one line python builtin for doing it properly: itertools.cycle(iterable_thing))

Also refactored lots of modern python usage like using pathlib.Path for easy and more semantic file manipulation instead of a dozen different os.path functions.

and of course complying with modern syntax means we need to use the standard python code formatter instead of bespoke "data-science-ese" of write-once disposable code patterns (but it still gets copied in thousands of places and just ends up decaying more and more over time)

also added a pyproject/poetry config to specify requirements somewhat (and as a bonus, i configured current scripts as poetry runnables so you can just do things like poetry run adder and it works automatically)

good luck.

mattsta avatar Jul 24 '22 20:07 mattsta

I appreciate the effort, I reviewed the full diff, but I don't like it. But I do like the itertools.cycle call, which I was not originally aware of and I think may want to adopt.

karpathy avatar Jul 25 '22 18:07 karpathy

Actually on closer look I'm a bit worried about itertools.cycle, as it caches the individual elements in memory to yield through them on second pass. I am worried this would consume a large amount of memory, which the current implementation discards and recreates each element of the data loader on demand. Punting for now.

karpathy avatar Jul 25 '22 22:07 karpathy

oh, good catch. could also just do it manually with a hand rolled infinite generator:

In [1]: import random

In [2]: randos = [random.randbytes(3000) for i in range(333)]

In [3]: def forever():
   ...:     while True:
   ...:         yield from randos

In [4]: for i, got in enumerate(forever()):
   ...:     print(i)
   ...:     if i > 3000000:
   ...:         break

mattsta avatar Jul 26 '22 00:07 mattsta