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

[tuner] Convert compile_candidate.sh to py function compile_dispatch() #96

Closed
wants to merge 2 commits into from

Conversation

RattataKing
Copy link
Contributor

@RattataKing RattataKing commented Aug 13, 2024

Bash scripts will be converted as functions in autotune_functions.py
Each function will have a TaskPack and ResultPack as input and output

autotune_functions.py:

  • compile_dispatch() compiles a single dispatch

autotune.py:

  • Add class CompilationMode to replace the original list ['default', 'winograd']
  • Refactor compile_candidates()
  • Edit var name and append new dir path in PathConfig
  • Add CompileDispatchResultPack maker function
  • Add function to write #_spec.mlir for candidates

@RattataKing RattataKing requested a review from kuhar August 13, 2024 13:24
Copy link
Member

@kuhar kuhar left a comment

Choose a reason for hiding this comment

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

Seeing bash go away makes me happy!

I think this PR attempts to do two things at once:

  1. Reduce the amount of bash
  2. Make tuning less coupled to the exact model (Punet).

I think it would be easier to move forward if we focused on doing only one of these at a time. If we start with 2., we can define an interface for a model-specific tuners and still use bash scripts to implement that. Once that interface is in place, we can start removing bash.

The reason I think this should work out better is that the alternative is that we will come up with some short-lived python API for compilation/benchmarking that may not even survive the refactoring to make the tuner less coupled to punet.

The third alternative is to do both at the same time. This would also work, but I'm afraid it would be the most difficult to review.

@@ -20,6 +20,8 @@
import pickle
import iree.runtime as ireert
import random
import autotune_functions
Copy link
Member

Choose a reason for hiding this comment

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

How about we rename autotune.py to punet_autotune.py and then move the reusable code to an empty autotune.py?

Comment on lines +160 to +162
class CompilationMode(Enum):
DEFAULT = "default"
WINOGRAD = "winograd"
Copy link
Member

Choose a reason for hiding this comment

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

We don't need it anymore, punet doesn't use winograd

Comment on lines +809 to +816
prolog_content = prolog_file.read()
config_content = config_file.read()
epilog_content = epilog_file.read()

# Write the contents to the spec file
spec_file.write(prolog_content)
spec_file.write(config_content)
spec_file.write(epilog_content)
Copy link
Member

Choose a reason for hiding this comment

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

This is fine for now, but in the next iteration we should use the logic in generate_config.py instead.

multiprocess_progress_wrapper(
num_worker=num_worker, task_list=task_list, function=run_command_wrapper
task_list = make_CompileDispatchTaskPack_task_list(args, path_config, candidates)
num_worker = max(min(args.max_cpu_workers, len(candidates)), 1)
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
num_worker = max(min(args.max_cpu_workers, len(candidates)), 1)
num_workers =min(args.max_cpu_workers, len(candidates))

If there are no candidates, we should exit

assert input_path is not None
out_path = path_config.get_candidate_vmfb_path(candidate_id)

if result.success == 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 result.success == False:
if not result.success:


if result.success == False:
failed_candidates.append(candidate_id)
# move candidates that failed to compile to a different dir. (i.e. candidates/compiled/#.mlir -> candidates/failed/#.mlir)
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
# move candidates that failed to compile to a different dir. (i.e. candidates/compiled/#.mlir -> candidates/failed/#.mlir)
# Move candidates that failed to compile to a different dir. (i.e. candidates/compiled/#.mlir -> candidates/failed/#.mlir)

out_path.unlink()
continue

# collect hash val for compiled files
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
# collect hash val for compiled files
# Collect hash values for all compiled files

@kuhar kuhar marked this pull request as draft August 19, 2024 19:09
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