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.

Sign up for GitHub

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

Jump to bottom

Update rocm6.2_internal_testing branch with changes from rocm6.1_internal_testing branch #52

Merged
merged 20 commits into from
Apr 24, 2024
Merged

Update rocm6.2_internal_testing branch with changes from rocm6.1_internal_testing branch #52

merged 20 commits into from
Apr 24, 2024

Conversation

shbiswas834
Copy link

No description provided.

pnunna93 and others added 17 commits February 27, 2024 20:55
@pnunna93
Skip pytest parameterize check for pytest > 7.4.2
a05f318
Enable ROCm signal iir utils
cbeb5f5
Revert "Enable ROCm signal iir utils"
b5b9a5d 
This reverts commit cbeb5f5.
Enable ROCm signal iir utils
0b307bc
Fix scipy window compilation errors
5cb9d0b
Update UT status
d39423c
Update UT status
2c53c4f
Fix flake8 issues
c116fed
@AdrianAbeyta
Merge pull request #46 from ROCm/rocm6.1_SWDEV-446431
9134a29 
Fix scipy signal tests for ROCm (SWDEV-446431)
@lcskrishna
fix the parser for test summary
979257e
@lcskrishna
fix parser for test summary
efbce25
@AdrianAbeyta
Merge pull request #47 from lcskrishna/cl/fix-swdev-446433
f49d65e 
[SWDEV-446433] Fix the unit test parser in CuPy
@lcskrishna
fix the parser
da2e426
@AdrianAbeyta
Merge pull request #48 from lcskrishna/cl/fix-swdev-446433
085ff62 
Fix the parser in run_tests_rocm.py
@shbiswas834
Merge remote-tracking branch 'origin/rocm6.1_internal_testing' into m…
5e3907d 
…erge-6.1-to-6.2
@shbiswas834
Resolved merge conflicts from 6.1
f49cd32
@shbiswas834
Hipsolver issue with hipsolverDnParams_t
abdea1a
@lcskrishna
Copy link

Hi @shbiswas834 could you please check build failures in the CI.

lcskrishna
lcskrishna requested changes Apr 20, 2024
View reviewed changes
Copy link

@lcskrishna lcskrishna left a comment

Choose a reason for hiding this comment

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

Few minor changes might be needed to take care of versioning stuff. Otherwise thanks for creating the PR, it looks good.

cupy_backends/hip/cupy_hipsolver.h Show resolved Hide resolved
cupy_backends/hip/cupy_hipsolver.h Outdated

#if HIP_VERSION < 50631061
#if HIP_VERSION < 60240094
Copy link

Choose a reason for hiding this comment

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

Double check this one again, same as above.

Copy link
Author

Choose a reason for hiding this comment

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

Addressed this above

@lcskrishna lcskrishna requested a review from amathews-amd April 20, 2024 05:33
@shbiswas834
Copy link
Author

@lcskrishna @AdrianAbeyta Please note the updated changes. I have updated this branch where I moved hipsolverDnParams_t under HIP_VERSION < 5063106. Verified the use of this variable from the rocm5.0 docker used for build tests.

@shbiswas834 shbiswas834 self-assigned this Apr 22, 2024
@shbiswas834
Copy link
Author

shbiswas834 commented Apr 23, 2024 •
edited
Loading

Cupy unit test suit logs

@AdrianAbeyta

lcskrishna
lcskrishna requested changes Apr 23, 2024
View reviewed changes
Copy link

@lcskrishna lcskrishna left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Could you please update these final changes? Else everything looks good. After fixing the builds on all the versions, let's focus on unit-test cases separately.

cupy_backends/hip/cupy_hipsolver.h Show resolved Hide resolved
@shbiswas834
Copy link
Author

shbiswas834 commented Apr 24, 2024 •
edited
Loading

Updates:

  • Initial error: ROCm 6.2 throws this error if hipsolverDnParams_t is declared
    image
  • hipsolverDnParams_t is required HIP_VERSION < 60240092 hence encased it under single condition
  • HIP_VERSION acquired from docker container
  • updated condition checked in dockers with ROCm 5.0, 6.1 & 6.2 with successful builds

@lcskrishna
Copy link

@shbiswas834 we need to update conditions for only hipsolverDnParams_t. I have updated the code.

@AdrianAbeyta AdrianAbeyta requested a review from lcskrishna April 24, 2024 21:13
AdrianAbeyta
AdrianAbeyta approved these changes Apr 24, 2024
View reviewed changes
Copy link

@AdrianAbeyta AdrianAbeyta left a comment

Choose a reason for hiding this comment

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

LGTM, all checks now passing. Thanks @shbiswas834 @lcskrishna

@AdrianAbeyta AdrianAbeyta merged commit cb44155 into rocm6.2_internal_testing Apr 24, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AdrianAbeyta AdrianAbeyta AdrianAbeyta approved these changes

@amathews-amd amathews-amd Awaiting requested review from amathews-amd

@lcskrishna lcskrishna Awaiting requested review from lcskrishna

Assignees

@shbiswas834 shbiswas834

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@shbiswas834 @lcskrishna @AdrianAbeyta @pnunna93