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

[Rework] Add checks on workspace params #2539

Open
junliume opened this issue Nov 18, 2023 · 2 comments
Open

[Rework] Add checks on workspace params #2539

junliume opened this issue Nov 18, 2023 · 2 comments

Comments

@junliume
Copy link
Collaborator

@amberhassaan I have to revert #2498 for now but the work is still valuable even critical so we would like to update that feature branch further before we can merge another PR on it
CC: @JehandadKhan @atamazov

[Observations}:
CI: http://micimaster.amd.com/blue/organizations/jenkins/MLLibs%2FMIOpen/detail/develop/1265/pipeline
Also observed on gfx1100:
LastTest.log

It appears that the value of workSpaceSize is not updated from the latest evaluated solver:

  • In this case, after "[FindSolutionImpl] ConvDirectNaiveConvWrw" with "[ValidateWorkspace] workSpace = 0x7f6ea4607000, workSpaceSize = 8192", eventually the chosen one "[FindConvFwdAlgorithm] FW Chosen Algorithm: ConvDirectNaiveConvFwd" which should have "[ValidateWorkspace] workSpace = 0, workSpaceSize = 0"
  • However, workSpace seems to be updated while workSpaceSize is not updated from the last value, so it stayed at 8192 in "[ValidateWorkspace] workSpace = 0, workSpaceSize = 8192"
  • This will cause the workSpace check to fail
@amberhassaan
Copy link
Contributor

@junliume : I'll take a look. It seems the check did its job :). I have a fix in progress here and I'll make sure it handles the case above too: #2524

@atamazov
Copy link
Contributor

[Notice] @junliume

the work is still valuable even critical

This is definitely valuable! but I am not sure if this is critical or not. Maybe there are (or were) some P0 tickets related to incorrect workspace ptr/size passed by the client to the library?

I have to revert #2498 for now

That's exactly the case I meant in #2498 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants