Skip to content

Benchmarks: add TIMES NZ, IE, and eTIMES-EU benchmarks#96

Merged
siddharth-krishna merged 16 commits intomainfrom
daniele/times
Apr 3, 2025
Merged

Benchmarks: add TIMES NZ, IE, and eTIMES-EU benchmarks#96
siddharth-krishna merged 16 commits intomainfrom
daniele/times

Conversation

@danielelerede-oet
Copy link
Member

Hi @siddharth-krishna , Victor's TIMES benchmarks added with metadata (with Contributor line) and update of benchmark_config.yaml

@vercel
Copy link

vercel bot commented Feb 13, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
solver-benchmark ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 3, 2025 1:33pm

@danielelerede-oet danielelerede-oet changed the title add Victor's TIMES benchmarks add TIMES benchmarks Feb 13, 2025
@danielelerede-oet
Copy link
Member Author

added TIMES-NZ benchmarks @siddharth-krishna , I'm updating the slides, too

Copy link
Member

@siddharth-krishna siddharth-krishna left a comment

Choose a reason for hiding this comment

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

Thanks, Daniele. I requested some minor changes. I'll merge #99 first, so it would be good if you could also indicate the sizes, or at least which ones count as "real" -- I would guess all of them?

@danielelerede-oet
Copy link
Member Author

Hi @siddharth-krishna , I made the changes you asked

Copy link
Member

@siddharth-krishna siddharth-krishna left a comment

Choose a reason for hiding this comment

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

Hi Daniele, apologies I dropped the ball on this PR and now it's stale and needs to be updated with the main branch (and the info from benchmark_config.yaml needs to be merged into the metadata.yaml file, e.g. URL). Would you have time to do this?

Other remaining TODOs:

  • Remove Olex's affiliation
  • The 2 new comments I've added inline

@siddharth-krishna siddharth-krishna changed the title add TIMES benchmarks Add TIMES benchmarks Mar 20, 2025
Co-authored-by: Siddharth Krishna <siddharth-krishna@users.noreply.github.com>
This reverts commit db13fa4, reversing
changes made to 38b295e.
@danielelerede-oet
Copy link
Member Author

Hi @siddharth-krishna , I've done all the changes, though I was having issues with merging the main branch. The metadata file is completely revised though. If there are any problems I could save the file locally and open a new PR generating the new branch from the current main, let me know please :)

@siddharth-krishna
Copy link
Member

siddharth-krishna commented Apr 3, 2025

Thanks, @danielelerede-oet . I fixed a syntax error and ran the merge_metadata.py script. I am able to run these benchmarks successfully (though with a 10s timeout, so they all timed out). I think the only remaining thing is to decide the size category for these new benchmarks. Should we mark them all as R, so they are run with a 10h timeout? Or should we run them with a 1h timeout and see if they can be solved quicker?

I didn't see any merge issues. But if you could double check that the diff in https://github.com/open-energy-transition/solver-benchmark/pull/96/files has the latest metadata, that would be great. It looks good to me, but I can't check if the benchmark metadata has been categorized correctly.

@danielelerede-oet
Copy link
Member Author

danielelerede-oet commented Apr 3, 2025

@siddharth-krishna everything's fine with the diff.
They should all be solved within 1 hour, even though they can be considered as "real" benchmarks from the modelling point of view.
Also, could you please check that the "Name" format with "ts" instead of "h" works well? In case yes, I should make the modifications on some other benchmarks, too, but probably better to open a new PR once all the benchmarks are merged to the main branch

Copy link
Member

@siddharth-krishna siddharth-krishna left a comment

Choose a reason for hiding this comment

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

Thanks, Daniele, I think with the new website you can have anything in the "Name:" field, it doesn't have to follow the old format of <spatial_res>-<temporal_res>. Please do open a new PR updating all the metadata as you suggest, and we'll double check in that branch that the website continues to work fine.

@siddharth-krishna siddharth-krishna changed the title Add TIMES benchmarks Benchmarks: add TIMES NZ, IE, and eTIMES-EU benchmarks Apr 3, 2025
@siddharth-krishna siddharth-krishna enabled auto-merge (squash) April 3, 2025 13:36
@siddharth-krishna siddharth-krishna merged commit 908a114 into main Apr 3, 2025
3 of 4 checks passed
@siddharth-krishna siddharth-krishna deleted the daniele/times branch April 3, 2025 13:36
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