Conversation
|
Draft PR needs ProteinGym/proteingym-base#310 to complete |
JCZuurmond
left a comment
JCZuurmond
left a comment
There was a problem hiding this comment.
The changes require the rename of pg2-dataset plus a new release
| @@ -2,11 +2,11 @@ | |||
| from typing import Annotated | |||
There was a problem hiding this comment.
Note that models are now under proteingym.models
| git config --global credential.helper store && \ | ||
| cat /run/secrets/git_auth > ~/.git-credentials && \ | ||
| chmod 600 ~/.git-credentials | ||
| # TODO: After proteingym-base and proteingym-benchmark are public repositories, below COPY can be removed |
There was a problem hiding this comment.
@tintinrevient : See this first step to simplifying the Dockerfile. For now, we will download the latest versions of proteingym-base and proteingym-benchmark into dist/ before running the docker build. Later we will use public packages from PyPi (or Github)
| train_X, train_Y = load_x_and_y( | ||
| dataset=dataset, | ||
| split=TrainTestValid.train, | ||
| split=1, # TODO: Update split |
There was a problem hiding this comment.
Postponing this until the split work is done
| @@ -1,49 +1,51 @@ | |||
| # Model | |||
There was a problem hiding this comment.
Reviewed, it looks good! Only the link []() is relative to models.
| - [models/esm/src/proteingym/models/esm/__main__.py] | ||
| - [models/pls/src/proteingym/models/pls/__main__.py] |
There was a problem hiding this comment.
| - [models/esm/src/proteingym/models/esm/__main__.py] | |
| - [models/pls/src/proteingym/models/pls/__main__.py] | |
| - [esm/src/proteingym/models/esm/__main__.py](esm/src/proteingym/models/esm/__main__.py) | |
| - [esm/src/proteingym/models/esm/__main__.py](esm/src/proteingym/models/esm/__main__.py) |
JCZuurmond
force-pushed
the
rename-package-for-clarity
branch
from
515bffa to
d175db7
Compare
|
The only one thing is the CML pipeline is failing because in docker build --build-arg GIT_CACHE_BUST=${git.git_cache_bust} --secret id=git_auth,src=../git-auth.txt ...It still looks for But I feel, we can overlook CML pipeline for a while, before everything is cleaned up, then we make it work again on a clean slate. |
tintinrevient
left a comment
tintinrevient
left a comment
There was a problem hiding this comment.
Overall, it looks good to me and approve this. CML pipeline can be fixed later, since this is already a big change.
|
I will merge this one, and after clean up make the CML work again. |
2 participants
Changes
Progresses #152
Rename
pg2-benchmarktoproteingym.benchmarkin various places:pyproject.tomlRename
pg2-modeltoproteingym.benchmark.modelsin various places:pyproject.tomlChecklist