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

Added quickTunerGen.py, the main driver of the quick tuner scripts #1576

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

ethansaurusrex
Copy link

Generates quick tuner perf configs given the dataframe/tsv file produced by quickTunerPreproc.py.

which generates quick tuner perf configs given the dataframe/tsv
file produced by quickTunerPreproc.py.
Copy link

codecov bot commented Jul 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.40%. Comparing base (3b4956b) to head (9d4fddf).
Report is 43 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1576      +/-   ##
===========================================
- Coverage    77.51%   77.40%   -0.12%     
===========================================
  Files           90       92       +2     
  Lines        24358    24580     +222     
  Branches      3414     3451      +37     
===========================================
+ Hits         18881    19026     +145     
- Misses        4132     4189      +57     
- Partials      1345     1365      +20     
Flag Coverage Δ
mfma 77.40% <ø> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@krzysz00 krzysz00 left a comment

Choose a reason for hiding this comment

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

Might as well get the complaining in before you leave - sorry to drop this on the last minute

as input.
Needs the input to be a combined normalized dataframe (default from quickTunerPreproc.py)

Usage: clusterConfigs.py [-h] --input-file INPUT_FILE [--method {default,topNSelect,topMode,takeNEach,fairSelect,hardcoded} [{default,topNSelect,topMode,takeNEach,fairSelect,hardcoded} ...]] [--save] [--debug] [--num NUM] [--perfconfig--format]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Stale name in docment

os.environ['OMP_NUM_THREADS'] = '1'
import sys

sys.path.append('../..')
Copy link
Collaborator

Choose a reason for hiding this comment

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

... ???

You shouldn't need to do this

import re
import glob

class quickTunerMethod(object):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same case/(object) nits as on other PRs


trans_cols = ['TransA', 'TransB']

param_cols = [ 'G', 'M', 'N','K']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit, weird newlines



self.default_f16 = pd.DataFrame({
"M/block": [128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 32, 32, 32, 32, 32, 32, 16, 16, 16, 16],
Copy link
Collaborator

Choose a reason for hiding this comment

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

.. what's this about? Why do we have this? Where are the notes to keep it up to date?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, if you're doing to do this, build the dataframe from a list of lists and give the column names separately - this is unreadable.


if __name__ == '__main__':
main(sys.argv[1:])

Copy link
Collaborator

Choose a reason for hiding this comment

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

Something feels really awkward here - I can't for the life of me follow what all this is supposed to be doing and there's a whole lot of for loops and manual bouncing around of columns

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