Skip to content

clean normalization_inner_outer#928

Merged
liqiangxl merged 16 commits intomainfrom
llu/clean_inner_outer_norm
Sep 27, 2023
Merged

clean normalization_inner_outer#928
liqiangxl merged 16 commits intomainfrom
llu/clean_inner_outer_norm

Conversation

@liqiangxl
Copy link
Collaborator

@liqiangxl liqiangxl commented Sep 22, 2023

similar to #923
(1) Transformed private static functions in the InnerOuterPersistentKernelScheduler class into utility functions within an anonymous namespace.
(2) Removed checks/calculations that are unrelated to inner_outer persistent scheduler.

@liqiangxl
Copy link
Collaborator Author

!build

@liqiangxl
Copy link
Collaborator Author

!build

@liqiangxl
Copy link
Collaborator Author

!build

@liqiangxl liqiangxl marked this pull request as ready for review September 23, 2023 16:48
@liqiangxl liqiangxl requested a review from naoyam September 23, 2023 16:50
@liqiangxl
Copy link
Collaborator Author

Checked benchmark cases using tools/compare_codegen.sh -- build/nvfuser_bench --benchmark_filter=NvFuserScheduler --benchmark_repetitions=1 --benchmark_min_time=0

Previous HEAD position was b3051d8f Fix a memory leak. (#936)
Switched to branch 'llu/clean_inner_outer_norm'
Your branch is up to date with 'origin/llu/clean_inner_outer_norm'.


DIFF RESULT:

No difference found

@liqiangxl
Copy link
Collaborator Author

!build

@liqiangxl
Copy link
Collaborator Author

!build

@liqiangxl
Copy link
Collaborator Author

!build

@liqiangxl
Copy link
Collaborator Author

!build

@liqiangxl
Copy link
Collaborator Author

!build


return rparams;
}
auto getBlocksPerSM = [&](const int64_t threads_per_sm,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This part shows as changed. Is there any change here?

Github's diff indicates lots of changes. Maybe it's just false positives? Could you try to minimize the (false?) changes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No change. I tried hide whitespace and it removes false changes.
image

@liqiangxl
Copy link
Collaborator Author

!build

Copy link
Collaborator

@naoyam naoyam left a comment

Choose a reason for hiding this comment

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

LGTM

@naoyam
Copy link
Collaborator

naoyam commented Sep 26, 2023

Do you know why the CI diff check shows some diffs?

@liqiangxl
Copy link
Collaborator Author

liqiangxl commented Sep 27, 2023

Do you know why the CI diff check shows some diffs?

The reference used in diff compare is Commits on Sep 24, 2023:
HEAD is now at 4004b72 Store compiled CUDA kernel in Flatbuffers binary (#814)

(1) nvfuser-ci/jit_codegen_diff_10/10 is because more benchmark cases are ran by CI after #930 which is merged on Sep 25.
(2) nvfuser-ci/jit_codegen_diff_5/10 is fixed by #947 which is merged on Sep 26.

@liqiangxl liqiangxl merged commit 3670202 into main Sep 27, 2023
@liqiangxl liqiangxl deleted the llu/clean_inner_outer_norm branch September 27, 2023 01: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.

2 participants