search

JasonGross / guarantees-based-mechanistic-interpretability

MIT License
8 stars 2 forks source link

Should we commit poetry.lock? #16

Closed JasonGross closed 9 months ago

JasonGross commented 9 months ago

https://github.com/JasonGross/guarantees-based-mechanistic-interpretability/blob/d87b9c746bcdb3e1c81d8f5b2f80dd987e6ca7aa/.gitignore#L97-L102 Maybe we should remove https://github.com/JasonGross/guarantees-based-mechanistic-interpretability/blob/d87b9c746bcdb3e1c81d8f5b2f80dd987e6ca7aa/.gitignore#L161 ?

Questions:

  1. Will this break cross-platform development?
  2. Do we want to have CI test on multiple versions of python, and will having a lock file interfere with this?
  3. Do we want to have CI test multiple versions of poetry, and will having a lock file interfere with this?

Currently CI tests https://github.com/JasonGross/guarantees-based-mechanistic-interpretability/blob/d87b9c746bcdb3e1c81d8f5b2f80dd987e6ca7aa/.github/workflows/ci.yml#L18-L28

If we change this, presumably we'll want to add hashFiles('poetry.lock') to https://github.com/JasonGross/guarantees-based-mechanistic-interpretability/blob/d87b9c746bcdb3e1c81d8f5b2f80dd987e6ca7aa/.github/workflows/ci.yml#L58 and maybe remove some subset of the existing key.

euanong commented 9 months ago

see https://python-poetry.org/docs/basic-usage/ -- seems to suggest committing the lock makes sense?

JasonGross commented 9 months ago

Okay, let's try it. I'm concerned that the lockfile is going to pin some pytorch cuda packages specific to some particular GPU and the lockfile for the node / MacOS / WSL / Linux are all going to conflict. Maybe it'll magically work, but we should keep an eye out.