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

Feature/benchmarks on windows #184

Merged
merged 17 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ name: CI
on:
# Triggers the workflow on push or pull request events but only for the main branch
push:
branches: [main]
branches: [ main ]
pull_request:
branches: [main]
branches: [ main ]

# Allows you to run this workflow manually from the Actions tab
workflow_dispatch:
Expand Down Expand Up @@ -69,6 +69,9 @@ jobs:
# ---------- Install GAMS

- name: Install GAMS
env:
GAMS_LICENSE: ${{ secrets.GAMS_LICENSE }}
if: ${{ env.GAMS_LICENSE != '' }}
run: |
curl https://d37drm4t2jghv5.cloudfront.net/distributions/44.1.0/linux/linux_x64_64_sfx.exe -o linux_x64_64_sfx.exe
chmod +x linux_x64_64_sfx.exe
Expand All @@ -81,17 +84,18 @@ jobs:
mkdir -p $HOME/.local/share/GAMS
echo "$GAMS_LICENSE" > $HOME/.local/share/GAMS/gamslice.txt
ls -l $HOME/.local/share/GAMS/
env:
GAMS_LICENSE: ${{ secrets.GAMS_LICENSE }}


# ---------- Run tool, check for regressions

- name: Run tool on all benchmarks
env:
GAMS_LICENSE: ${{ secrets.GAMS_LICENSE }}
if: ${{ env.GAMS_LICENSE != '' }}
working-directory: xl2times
# Use tee to also save the output to out.txt so that the summary table can be
# printed again in the next step.
# Save the return code to retcode.txt so that the next step can fail the action
# if run_benchmarks.py failed.
run: |
source .venv/bin/activate
export PATH=$PATH:$GITHUB_WORKSPACE/GAMS/gams44.1_linux_x64_64_sfx
Expand All @@ -101,6 +105,23 @@ jobs:
| tee out.txt; \
echo ${PIPESTATUS[0]} > retcode.txt)

- name: Run CSV-only regression tests (no GAMS license)
SamRWest marked this conversation as resolved.
Show resolved Hide resolved
env:
GAMS_LICENSE: ${{ secrets.GAMS_LICENSE }}
if: ${{ env.GAMS_LICENSE == '' }}
working-directory: xl2times
# Run without --dd flag GAMS license secret doesn't exist to see if we're just missing a GAMS license.
# This way we still run CSV-only regression tests without GAMS - useful for testing in forks before creating PRs.
siddharth-krishna marked this conversation as resolved.
Show resolved Hide resolved
run: |
source .venv/bin/activate
export PATH=$PATH:$GITHUB_WORKSPACE/GAMS/gams44.1_linux_x64_64_sfx
(python utils/run_benchmarks.py benchmarks.yml \
--times_dir $GITHUB_WORKSPACE/TIMES_model \
--verbose \
| tee out.txt; \
echo ${PIPESTATUS[0]} > retcode.txt)
echo 'Note: Pipeline will fail in final step due to missing GAMS license'
siddharth-krishna marked this conversation as resolved.
Show resolved Hide resolved

- name: Print summary
working-directory: xl2times
run: |
Expand Down
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ ground_truth/*
*.pyproj.*
speedscope.json
*.pkl
.venv/
.venv*/
benchmarks/
.idea/
docs/_build/
docs/api/
1 change: 1 addition & 0 deletions .python-version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
3.11.4
siddharth-krishna marked this conversation as resolved.
Show resolved Hide resolved
38 changes: 38 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,44 @@ python -m build
python -m twine upload dist/*
```

In summary, use the commands below to clone the benchmarks data into your local `benchmarks` dir.
Note that this assumes you have access to all these repositories (some are private and
you'll have to request access) - if not, comment out the inaccessible benchmarks from `benchmakrs.yml` before running.

```bash
# Get VEDA example models and reference DD files
SamRWest marked this conversation as resolved.
Show resolved Hide resolved
# XLSX files are in private repo for licensing reasons, please request access or replace with your own licensed VEDA example files.
git clone [email protected]:olejandro/demos-xlsx.git benchmarks/xlsx/
git clone [email protected]:olejandro/demos-dd.git benchmarks/dd/

# Get Ireland model and reference DD files
git clone [email protected]:esma-cgep/tim.git benchmarks/xlsx/Ireland
git clone [email protected]:esma-cgep/tim-gams.git benchmarks/dd/Ireland
```
Then to run the benchmarks:
```bash
# Run a only a single benchmark by name (see benchmarks.yml for name list)
python utils/run_benchmarks.py benchmarks.yml --verbose --run DemoS_001-all

# Run all benchmarks (without GAMS run, just comparing CSV data)
python utils/run_benchmarks.py benchmarks.yml --verbose | tee out.txt
SamRWest marked this conversation as resolved.
Show resolved Hide resolved


# Run benchmarks with regression tests vs main branch
git branch feature/your_new_changes --checkout
# ... make your code changes here ...
git commit -a -m "your commit message" # code must be committed for comparison to `main` branch to run.
python utils/run_benchmarks.py benchmarks.yml --verbose | tee out.txt
```
At this point, if you haven't broken anything you should see something like:
```
Change in runtime: +2.97s
Change in correct rows: +0
Change in additional rows: +0
No regressions. You're awesome!
```
If you have a large increase in runtime, a decrease in correct rows or fewer rows being produced, then you've broken something and will need to figure out how to fix it.

SamRWest marked this conversation as resolved.
Show resolved Hide resolved
## Contributing

This project welcomes contributions and suggestions. Most contributions require you to agree to a
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ dependencies = [
"more-itertools",
"openpyxl >= 3.0, < 3.1",
"pandas >= 2.1",
"pyarrow"
SamRWest marked this conversation as resolved.
Show resolved Hide resolved
]

[project.optional-dependencies]
Expand Down
8 changes: 6 additions & 2 deletions utils/dd_to_csv.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,17 @@ def convert_dd_to_tabular(
return


if __name__ == "__main__":
def main(arg_list: None | list[str] = None):
SamRWest marked this conversation as resolved.
Show resolved Hide resolved
args_parser = argparse.ArgumentParser()
args_parser.add_argument(
"input_dir", type=str, help="Input directory containing .dd files."
)
args_parser.add_argument(
"output_dir", type=str, help="Output directory to save the .csv files in."
)
args = args_parser.parse_args()
args = args_parser.parse_args(arg_list)
convert_dd_to_tabular(args.input_dir, args.output_dir, generate_headers_by_attr())


if __name__ == "__main__":
main()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we not need to pass in an arg_list here? (I haven't seen this idiom before)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sam, do you need to make this change here too?

Suggested change
main()
main(sys.argv[1:])

Loading
Loading