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

Jump to bottom

sci-libs/rocBLAS: various fixes #23450

Closed
wants to merge 4 commits into from
Closed

sci-libs/rocBLAS: various fixes #23450

wants to merge 4 commits into from

Conversation

littlewu2508
Copy link
Contributor

This PR contains three commits:

let TensileCreateLibrary respects MAKEOPTS
Closes: https://bugs.gentoo.org/822828

common fix for unuesd cmake variable:
Bugs: https://bugs.gentoo.org/829326

fix test failures:
Closes: ROCm/rocBLAS#1202

@littlewu2508
sci-libs/rocBLAS: let TensileCreateLibrary respects MAKEOPTS
58e819f 
Originaly TensileCreateLibrary consumes all cpu detected. This
modification will inject $(makeopts_jobs) into TensileCreateLibrary.py so
it respects MAKEOPTS

Closes: https://bugs.gentoo.org/822828
Package-Manager: Portage-3.0.22, Repoman-3.0.3
Signed-off-by: Yiyang Wu <xgreenlandforwyy@gmail.com>
@littlewu2508
sci-libs/rocBLAS: remove unused cmake variable
307e678 
__skip_rocmclang is used to avoid configuration error for
cmake-3.21.(1|2), which don't exist among ebuilds anymore, so this flag
is not recognized.

Package-Manager: Portage-3.0.22, Repoman-3.0.3
Signed-off-by: Yiyang Wu <xgreenlandforwyy@gmail.com>
@littlewu2508
sci-libs/rocBLAS: fix tests using openblas as cpu reference
35008de 
Add a patch that classify 6 test suites as known_bug.
Upstream will fix this issue in future releases,
and they recommand this as solution for current version.

Closes: ROCm/rocBLAS#1202
Package-Manager: Portage-3.0.22, Repoman-3.0.3
Signed-off-by: Yiyang Wu <xgreenlandforwyy@gmail.com>
@gentoo-bot
Copy link

Pull Request assignment

Submitter: @littlewu2508
Areas affected: ebuilds
Packages affected: sci-libs/rocBLAS

sci-libs/rocBLAS: @gentoo/sci, @justxi, @littlewu2508

Linked bugs

Bugs linked: 822828

In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of Conduct ● Copyright policy (expl.) ● Devmanual ● GitHub PRs ● Proxy-maint guide

@gentoo-bot gentoo-bot added self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Dec 21, 2021
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-12-21 04:05 UTC
Newest commit scanned: 35008de
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/6cf95d2efa/output.html

thesamesam
thesamesam reviewed Dec 21, 2021
View reviewed changes
sci-libs/rocBLAS/rocBLAS-4.3.0.ebuild
@@ -97,7 +99,6 @@ src_configure() {
-DBUILD_CLIENTS_TESTS=$(usex test ON OFF)
-DBUILD_CLIENTS_BENCHMARKS=$(usex benchmark ON OFF)
${AMDGPU_TARGETS+-DAMDGPU_TARGETS="${AMDGPU_TARGETS}"}
-D__skip_rocmclang="ON" ## fix cmake-3.21 configuration issue caused by officialy support programming language "HIP"
Copy link
Member

Choose a reason for hiding this comment

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

Depend on a newer CMake version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

only cmake-3.21.1 and cmake-3.21.2 need this, version below this or higher than this don't need or recognize this flag, and will throw a warning (like https://bugs.gentoo.org/829326 described). The two version does not exists in portage tree now.

@littlewu2508
sci-libs/rocBLAS: fix LICENSE
80a8f4a 
The rocBLAS license is BSD, but is mistakenly set to MIT in ebuild

Package-Manager: Portage-3.0.30, Repoman-3.0.3
Signed-off-by: Yiyang Wu <xgreenlandforwyy@gmail.com>
@littlewu2508
Copy link
Contributor Author

I found that the license is incorrect. rocBLAS is distributed under BSD.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2022-01-14 14:18 UTC
Newest commit scanned: 80a8f4a
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/583753ab75/output.html

@littlewu2508
Copy link
Contributor Author

@heroxbd Please have a look of this PR. I've got ROCm-5.0 ebuilds prepared, so it would be better if we apply existing fixes.

@AndrewAmmerlaan
Copy link
Member

I found that the license is incorrect. rocBLAS is distributed under BSD.

We need both, rocBLAS is BSD but Tensile is MIT.
By the way, is it at all possible to unbundle Tensile? There is an ebuild for stand-alone dev-util/Tensile in the ::science repository.

@littlewu2508
Copy link
Contributor Author

We need both, rocBLAS is BSD but Tensile is MIT.

Tensile is only needed during compilation (python scripts that read configs and call compilers for GPU code). I'm not sure whether the MIT license of Tensile matters.

By the way, is it at all possible to unbundle Tensile? There is an ebuild for stand-alone dev-util/Tensile in the ::science repository.

The Tensile in ::science is not up-to-date. However I can maintain the current version of Tensile, and it is possible to unbundle it if I do some hack.

gentoo-bot pushed a commit to gentoo/sci that referenced this pull request Mar 5, 2022
@AndrewAmmerlaan
dev-util/Tensile: add version 4.3.0
f1b7b45 
gentoo/gentoo#23450

Package-Manager: Portage-3.0.30, Repoman-3.0.3
Signed-off-by: Andrew Ammerlaan <andrewammerlaan@gentoo.org>
@AndrewAmmerlaan
Copy link
Member

The Tensile in ::science is not up-to-date. However I can maintain the current version of Tensile, and it is possible to unbundle it if I do some hack.

It is up to date now 😄

@AndrewAmmerlaan
Copy link
Member

I'm not sure whether the MIT license of Tensile matters.

Depends, are these files installed to the system? If so, the license matters, if not then we don't care.

@AndrewAmmerlaan
Copy link
Member

There's an || die missing here:

sed -r -e "/pip install/s:([^ \"\(]*python) -m pip install ([^ \"\)]*):\1 setup.py install --single-version-externally-managed --root / WORKING_DIRECTORY \2:g" -i cmake/virtualenv.cmake

@gentoo-bot gentoo-bot closed this in b7e9473 Mar 5, 2022
@AndrewAmmerlaan
Copy link
Member

Thanks 👍

I also moved Tensile into ::gentoo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thesamesam thesamesam thesamesam left review comments

Assignees
No one assigned
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else)
Projects
None yet
Milestone
No milestone
5 participants
@littlewu2508 @gentoo-bot @gentoo-repo-qa-bot @AndrewAmmerlaan @thesamesam