Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add private configs #1996

Merged
merged 121 commits into from
Dec 11, 2023
Merged

Add private configs #1996

merged 121 commits into from
Dec 11, 2023

Conversation

JosselinSomervilleRoberts
Copy link
Contributor

@JosselinSomervilleRoberts JosselinSomervilleRoberts commented Nov 10, 2023

Follow up to #1903 to add private configs.

@JosselinSomervilleRoberts JosselinSomervilleRoberts changed the base branch from main to joss-refactor-4-deployments November 18, 2023 00:00
Base automatically changed from joss-refactor-4-deployments to main November 18, 2023 00:37
Copy link
Collaborator

@yifanmai yifanmai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly good. Just a few changes needed. Thanks!

src/helm/benchmark/model_metadata_registry.py Show resolved Hide resolved
src/helm/benchmark/model_metadata_registry.py Show resolved Hide resolved
src/helm/benchmark/config_registry.py Show resolved Hide resolved
Copy link
Collaborator

@yifanmai yifanmai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One change suggested, looks good otherwise!

for model_metadata_path in args.model_metadata_paths:
register_model_metadata_from_path(model_metadata_path)
for model_deployment_paths in args.model_deployment_paths:
register_model_deployments_from_path(model_deployment_paths)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you keep these flags for now because it looks like some people are using it e.g. #2110

@RawthiL
Copy link

RawthiL commented Dec 7, 2023

Hi, I'm here from PR #2110 , I tried to install this branch (on a docker image)
pip install git+https://github.com/stanford-crfm/helm.git@joss-refactor-8-private

But I cannot execute the helm-run, I get an error saying ModuleNotFoundError: No module named 'helm.config'. Were there any requirements changes?

@yifanmai yifanmai merged commit af55ff5 into main Dec 11, 2023
9 checks passed
@yifanmai yifanmai deleted the joss-refactor-8-private branch December 11, 2023 22:18
@yifanmai
Copy link
Collaborator

But I cannot execute the helm-run, I get an error saying ModuleNotFoundError: No module named 'helm.config'. Were there any requirements changes?

I believe this was broken by #1903 and fixed in #2021

For the record, we're continuing this discussion in #2110.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants