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

Prevent format.sh from formatting triton #756

Merged
merged 2 commits into from
Jun 15, 2023
Merged

Conversation

lockshaw
Copy link
Collaborator

@lockshaw lockshaw commented Jun 8, 2023

Description of changes:

Have scripts/format.sh not format files in triton/

Related Issues:

Linked Issues:

  • Issue #

Issues closed by this PR:

  • Closes #

Before merging:

  • Did you update the flexflow-third-party repo, if modifying any of the Cmake files, the build configs, or the submodules?

@lockshaw lockshaw requested a review from goliaro June 8, 2023 03:30
@lockshaw
Copy link
Collaborator Author

lockshaw commented Jun 9, 2023

@gabrieleoliaro I assume no CI checks need to be changed along with this?

@goliaro
Copy link
Collaborator

goliaro commented Jun 12, 2023

@gabrieleoliaro I assume no CI checks need to be changed along with this?

No need! Btw, in the inference branch, we fixed this issue by changing the format.sh line to:

mapfile -t FILES < <(git ls-files | grep -E '\.(h|cc|cpp|cu)$' | grep -v '^triton')

not sure which way is better

@lockshaw
Copy link
Collaborator Author

It only matters in a few corner cases, but I think this way is slightly better. For example, if you have a file triton_dont_ignore_me.cc in the root of the repo,

$ git ls-files | grep -E '\.(h|cc|cpp|cu)$' | grep -v '^triton' | grep dont_ignore_me

will ignore it, while

$ git ls-files ':!:triton/**' '*.h' '*.cc' '*.cpp' '*.cu' '*.c' | grep dont_ignore_me
triton_dont_ignore_me.cc

correctly sees it.

But yes, this is a very contrived case 🙂

@lockshaw lockshaw enabled auto-merge (squash) June 15, 2023 21:48
@lockshaw lockshaw merged commit cf6ce58 into master Jun 15, 2023
goliaro added a commit that referenced this pull request Jun 17, 2023
* Fix bug in elementwise multiplication with broadcasting (#764)

* Fix multinode test (#766)

* Fix UCX multinode test (#768)

* fix

* fix 2

* Prevent format.sh from formatting triton (#756)

* [CI] - Increase timeout in multinode test (UCX & MPI) (#773)

* fix

* fix 2

* increase timeout

* Fix docker builds in CI (#774)

---------

Co-authored-by: Soumya Chatterjee <[email protected]>
Co-authored-by: Colin Unger <[email protected]>
@goliaro goliaro deleted the fix/no-format-triton branch July 19, 2023 19:24
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