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

Openblas0.2.20 #800

Closed
wants to merge 4 commits into from
Closed

Openblas0.2.20 #800

wants to merge 4 commits into from

Conversation

kiwifb
Copy link
Contributor

@kiwifb kiwifb commented Jul 25, 2017

Move the latest openblas available to the newly released 0.2.20. This new release includes all the patches that were applied to 0.2.19 - including the big clang-3.9 patch stored on the mirror.
This new version also supports more (and newer) cpus out of the box, which has been a problem I have seen crop up on a regular basis on other forum.

@kiwifb
Copy link
Contributor Author

kiwifb commented Jul 25, 2017

I limited the PR to move to 0.2.20. Is there any real valid reasons for us to continue carrying the older 0.2.14 and 0.2.15? Especially considering they require significant patching and won't support newer cpus?

@kiwifb
Copy link
Contributor Author

kiwifb commented Aug 14, 2017

In case that's what holding this up, I have updated my GPG key to reflect my new address. So now the commits are all "verified".

@kiwifb
Copy link
Contributor Author

kiwifb commented Aug 21, 2017

One commit has been included in the final push to release 0.2.20 that prevents openblas to build on platform not using glibc (OS X for example). See OpenMathLib/OpenBLAS#1258 (comment)

I will update the PR with upstream fix for the problem. It will be included in the next release, which for some reason, is slow coming.

@kiwifb
Copy link
Contributor Author

kiwifb commented Aug 21, 2017

Travis is not happy because of the new repoman which checks for https in SRC_URI. I have fixed it for openblas (all versions) in this PR but I haven't touched the other ebuilds for which it fails.

Copy link
Contributor

@heroxbd heroxbd left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the bump, especially for solving the header issue. Could you please rebase your branch to the current head, get rid of the merge commit and squash the https SRC_URI commit with the other SRC_URI one?

# avoid pic when compiling static libraries, so re-compiling
if use static-libs; then
emake clean
emake libs ${openblas_flags} NO_SHARED=1 NEED_PIC=
mv libopenblas* libs/ || die
mv libopenblas* libs/ ||??die
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

François, your editor has messed up die again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. This is quite incredible, is that space some kind of unicode?

Copy link
Contributor

Choose a reason for hiding this comment

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

What editor are you using? I will try to avoid it in the future ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use a mix of kwrite and nano. My money is on kwrite doing something weird, but that implies that there is something special about that space - or its surroundings - to start with.

@@ -21,8 +21,6 @@ DEPEND="${RDEPEND}
virtual/pkgconfig"

MULTILIB_WRAPPED_HEADERS=(
/usr/include/openblas/cblas.h
/usr/include/openblas/f77blas.h
/usr/include/openblas/openblas_config.h
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, that's cleaner! Could you please add a reference in the commit message to the solution of header problem in bugs.g.o?

- make -j 1 -C $(NETLIB_LAPACK_DIR) blas_testing
+ $(MAKE) -j 1 -C $(NETLIB_LAPACK_DIR) blas_testing
(cd $(NETLIB_LAPACK_DIR)/BLAS && cat *.out)

Copy link
Contributor

Choose a reason for hiding this comment

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

So.. this patch is for openblas-0.20, but the name openblas-0.2.19-MAKE.patch. Does it break the openblas-0.19 ebuild?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are misreading the commit. I remove the whole patch. In fact I remove 0.2.19 and replace it with 0.2.20.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I realized that you have deleted openblas-0.2.19. Could you please keep the openblas-0.2.19 for a while?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I will probably restart the branch from scratch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks again for your work!

@@ -24,7 +24,7 @@ RDEPEND="
abi_x86_64? (
!abi_x86_32? (
|| (
>=dev-cpp/eigen-3.1.4
>=dev-cpp/eigen-3.1.4[fortran]
sci-libs/atlas[fortran]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this from a different PR?

@kiwifb
Copy link
Contributor Author

kiwifb commented Aug 23, 2017

@heroxbd The header issue requires #802 to be merged first. Are you on top of that?

Package-Manager: Portage-2.3.8, Repoman-2.3.3
Package-Manager: Portage-2.3.6, Repoman-2.3.3
Package-Manager: Portage-2.3.6, Repoman-2.3.3
@kiwifb
Copy link
Contributor Author

kiwifb commented Aug 23, 2017

@heroxbd Are you going to do a sweep of the overlay to fix the https issue? If you do that, I'll wait for it to be done before recreating the branch.

@heroxbd
Copy link
Contributor

heroxbd commented Aug 23, 2017

The header issue requires #802 to be merged first. Are you on top of that?

I am digesting it and learning from you.

Are you going to do a sweep of the overlay to fix the https issue? If you do that, I'll wait for it to be done before recreating the branch.

Why is it important? Does 301 redirection a big problem?

@kiwifb
Copy link
Contributor Author

kiwifb commented Aug 23, 2017

the https things makes travis check fail for everyone. That's what I am concerned about. You cannot rely on travis as an extra check.

@kiwifb
Copy link
Contributor Author

kiwifb commented Aug 23, 2017

Actually once #802 is merged (one way or another). I should fix all the openblas ebuilds in the overlay. Which means they will all get a revbump.

@heroxbd
Copy link
Contributor

heroxbd commented Aug 23, 2017

Actually once #802 is merged (one way or another). I should fix all the openblas ebuilds in the overlay. Which means they will all get a revbump.

Sure. Please go ahead.

Please follow the guide at https://github.com/gentoo/sci/blob/master/CONTRIBUTING.md to properly rebase the commits.

…pper issue. Use https to make repoman happy.

Package-Manager: Portage-2.3.6, Repoman-2.3.3
@kiwifb
Copy link
Contributor Author

kiwifb commented Aug 23, 2017

Hum... I have redone the branch on the current master (minus #802 I had started before you merged it) but it still remembers the point at which I started the PR. Should I just do a new PR or will it do?

@kiwifb kiwifb deleted the openblas0.2.20 branch October 16, 2018 09:02
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.

5 participants