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

Use global config #218

Merged
merged 2 commits into from
Apr 18, 2023
Merged

Use global config #218

merged 2 commits into from
Apr 18, 2023

Conversation

ZzEeKkAa
Copy link
Contributor

@ZzEeKkAa ZzEeKkAa commented Apr 17, 2023

Use global config and remove json reads from the code. After these changes applied there should be minimal changes to add support for npbench.

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • If this PR is a work in progress, are you filing the PR as a draft?

@ZzEeKkAa ZzEeKkAa self-assigned this Apr 17, 2023
@ZzEeKkAa ZzEeKkAa changed the title Use global config WIP: Use global config Apr 17, 2023
@ZzEeKkAa ZzEeKkAa force-pushed the feature/use_global_config branch 4 times, most recently from ccb061f to 2521de0 Compare April 17, 2023 21:11
@ZzEeKkAa ZzEeKkAa changed the title WIP: Use global config Use global config Apr 17, 2023
@ZzEeKkAa ZzEeKkAa marked this pull request as ready for review April 17, 2023 22:26
@ZzEeKkAa ZzEeKkAa force-pushed the feature/use_global_config branch from 2521de0 to bc310f0 Compare April 17, 2023 22:28
@ZzEeKkAa ZzEeKkAa enabled auto-merge April 17, 2023 22:28
@ZzEeKkAa ZzEeKkAa requested a review from diptorupd April 17, 2023 22:28
dpbench/runner.py Outdated Show resolved Hide resolved
dpbench/runner.py Outdated Show resolved Hide resolved
@diptorupd
Copy link

diptorupd commented Apr 18, 2023

@ZzEeKkAa we need a better name for dpbench.config.C. How about default_configs?

@diptorupd
Copy link

@ZzEeKkAa Overall good progress. Few general comments:

  1. I am not a huge fan of having lots of functions defined in __init__.py. Can you refactor and move the functions in dpbench.config.__init__.py into another module.
  2. The functions currently in dpbench.config.__init__.py should have proper docstrings (use https://google.github.io/styleguide/pyguide.html#383-functions-and-methods as reference). We will then generate sphinx docs out of these docstrings as a "how-to" guide for external frameworks and benchmarks to be run using dpbench.
  3. We have an embarrassingly non-existent documentation site. Let us start by adding a rst page on how the dpbench.config module should be used to create 3rd-party frameworks for dpbench benchmarks, provide architecture-specific configs to run benchmarks, and even load external benchmarks.

@ZzEeKkAa
Copy link
Contributor Author

@ZzEeKkAa we need a better name for dpbench.config.C. How about default_configs?

I would go with dpbench.config.GLOBAL to avoid double config in the name.

@ZzEeKkAa ZzEeKkAa force-pushed the feature/use_global_config branch 3 times, most recently from 415de0c to 75b34f6 Compare April 18, 2023 15:29
@ZzEeKkAa ZzEeKkAa force-pushed the feature/use_global_config branch 3 times, most recently from 4295c4d to f7827d2 Compare April 18, 2023 16:58
@ZzEeKkAa ZzEeKkAa force-pushed the feature/use_global_config branch from f7827d2 to a99f014 Compare April 18, 2023 17:03
@ZzEeKkAa ZzEeKkAa force-pushed the feature/use_global_config branch from a99f014 to 4d89433 Compare April 18, 2023 17:21
@ZzEeKkAa ZzEeKkAa merged commit 5f8f3a4 into main Apr 18, 2023
@ZzEeKkAa ZzEeKkAa deleted the feature/use_global_config branch April 18, 2023 19:38
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