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

Leaderboard #3

Merged
merged 11 commits into from
Jun 24, 2024
Merged

Leaderboard #3

merged 11 commits into from
Jun 24, 2024

Conversation

VyrodovMikhail
Copy link
Collaborator

No description provided.

Copy link
Member

@SpirinEgor SpirinEgor left a comment

Choose a reason for hiding this comment

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

Scripts folder are used for standalone scripts, e.g. run parsing or build some plots. For backend and frontend, it's better to move to separate packages:

  • Create sub-packages in goat: parser, backend, frontend, database
  • Scripts only contain files to start all of them with proper arguments.

For example, in backend sub-package you define your logic with queue to evaluation and use database to store result. And in scripts you will define run_backend.py that recieve credentials of database and starts listening for events.

scripts/leaderboard_backend/build_script.sh Outdated Show resolved Hide resolved
scripts/leaderboard_backend/eval.py Outdated Show resolved Hide resolved
scripts/leaderboard_backend/database_helper.py Outdated Show resolved Hide resolved
scripts/leaderboard_web/database_helper.py Outdated Show resolved Hide resolved
scripts/leaderboard_web/src_display_css_html_js.py Outdated Show resolved Hide resolved
@VyrodovMikhail VyrodovMikhail force-pushed the leaderboard branch 13 times, most recently from c18774e to 05ef4fc Compare May 26, 2024 11:50
@VyrodovMikhail VyrodovMikhail force-pushed the leaderboard branch 3 times, most recently from 4549ab1 to 7d1ac7a Compare June 3, 2024 09:50
pip install -r requirements.txt -r requirements.dev.txt
- name: "black"
run: black . --check --diff --color
- name: "isort"
run: isort . --check --diff
- name: "mypy"
run: mypy
run: mypy --ignore-missing-imports
Copy link
Member

Choose a reason for hiding this comment

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

This already set in pyproject.toml

results = evaluator.simple_evaluate(model=lm, tasks=[taskname])

model_id = model_name.replace("/", "__")
Path(f"goat/backend/results/{model_id}").mkdir(exist_ok=True)
Copy link
Member

Choose a reason for hiding this comment

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

Store this path into variable, you have multiple reusage later


model_id = model_name.replace("/", "__")
Path(f"goat/backend/results/{model_id}").mkdir(exist_ok=True)
lm_eval_output_file = f"goat/backend/results/{model_id + '_lm_eval'}.json"
Copy link
Member

Choose a reason for hiding this comment

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

Why did you create folder with model_id, but store results in model_id_lm_eval.json. Probably, this json should be inside model directory

Comment on lines +12 to +16
POSTGRES_USER = os.environ.get("POSTGRES_USER")
POSTGRES_PASSWORD = os.environ.get("POSTGRES_PASSWORD")
POSTGRES_DB = os.environ.get("POSTGRES_DB")
POSTGRES_IP = os.environ.get("POSTGRES_IP")
POSTGRES_PORT = os.environ.get("POSTGRES_PORT")
Copy link
Member

Choose a reason for hiding this comment

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

Probably, this should be part of DatabaseHelper class with additional check that these variables set

Comment on lines 35 to 38
if val == "True":
return True
else:
return False
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if val == "True":
return True
else:
return False
return val == "True"

readme = "README.md"
license = { "text" = "Apache-2.0" }
requires-python = ">=3.8"
dependencies = [
Copy link
Member

Choose a reason for hiding this comment

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

Should we split dependencies by subprojects?

@SpirinEgor SpirinEgor merged commit 9ac477d into main Jun 24, 2024
2 checks passed
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.

2 participants