ENH: support for detecting libraries in several directories simultaneously #229

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
4 participants
@npinto
Contributor

npinto commented Mar 6, 2012

Cleaned up PR from the following discussion:
http://projects.scipy.org/numpy/ticket/993

Let me know what you think.

@rgommers

This comment has been minimized.

Show comment
Hide comment
@rgommers

rgommers Mar 6, 2012

Member

If you could squash the ENH and FIX commits it's even easier to review.

This looks like a welcome improvement. With enough testing, I don't see a real problem merging this.

Member

rgommers commented Mar 6, 2012

If you could squash the ENH and FIX commits it's even easier to review.

This looks like a welcome improvement. With enough testing, I don't see a real problem merging this.

@npinto

This comment has been minimized.

Show comment
Hide comment
@npinto

npinto Mar 6, 2012

Contributor

Awesome.

How do we go about testing this ?

Contributor

npinto commented Mar 6, 2012

Awesome.

How do we go about testing this ?

@rgommers

This comment has been minimized.

Show comment
Hide comment
@rgommers

rgommers Mar 6, 2012

Member

Well, we've got Windows and Debian-on-SPARC buildbots (for after the merge), I can test on OS X and Wine. The X11 part needed some fixing on Ubuntu recently, so that would be a good system to test on.

We need to find one or two people who use most or all of the options (Umfpack, X11, MKL). I assume you're one of those people?

Member

rgommers commented Mar 6, 2012

Well, we've got Windows and Debian-on-SPARC buildbots (for after the merge), I can test on OS X and Wine. The X11 part needed some fixing on Ubuntu recently, so that would be a good system to test on.

We need to find one or two people who use most or all of the options (Umfpack, X11, MKL). I assume you're one of those people?

@rgommers

This comment has been minimized.

Show comment
Hide comment
@rgommers

rgommers Mar 6, 2012

Member

Now you also squashed in the PEP8 fixes. That achieves the opposite of what I wanted. Do you still have the original branch? I meant squash the (ENH + FIX), keep the second commit with stylistic changes separate.

Member

rgommers commented Mar 6, 2012

Now you also squashed in the PEP8 fixes. That achieves the opposite of what I wanted. Do you still have the original branch? I meant squash the (ENH + FIX), keep the second commit with stylistic changes separate.

@charris

This comment has been minimized.

Show comment
Hide comment
@charris

charris Mar 6, 2012

Member

It may be possible to use git reflog for recovery, it should be good for a month or so... undo rebase

Member

charris commented Mar 6, 2012

It may be possible to use git reflog for recovery, it should be good for a month or so... undo rebase

@npinto

This comment has been minimized.

Show comment
Hide comment
@npinto

npinto Mar 7, 2012

Contributor

Thanks. git reset --hard ORIG_HEAD worked here. Let me push.

Contributor

npinto commented Mar 7, 2012

Thanks. git reset --hard ORIG_HEAD worked here. Let me push.

@npinto

This comment has been minimized.

Show comment
Hide comment
@npinto

npinto Mar 7, 2012

Contributor

Let me know if this is in the format you expected.

We need to find one or two people who use most or all of the options (Umfpack, X11, MKL). I assume you're one of those people?

Yes, it works for me on Gentoo with ICC 12.1, see e.g. (https://github.com/npinto/numpy-mkl-bootstrap).

Contributor

npinto commented Mar 7, 2012

Let me know if this is in the format you expected.

We need to find one or two people who use most or all of the options (Umfpack, X11, MKL). I assume you're one of those people?

Yes, it works for me on Gentoo with ICC 12.1, see e.g. (https://github.com/npinto/numpy-mkl-bootstrap).

@rgommers

This comment has been minimized.

Show comment
Hide comment
@rgommers

rgommers Mar 7, 2012

Member

These commits look good, thanks.

Member

rgommers commented Mar 7, 2012

These commits look good, thanks.

@rgommers

This comment has been minimized.

Show comment
Hide comment
@rgommers

rgommers Mar 7, 2012

Member

In the weekend I can test and review in detail.

Member

rgommers commented Mar 7, 2012

In the weekend I can test and review in detail.

@npinto

This comment has been minimized.

Show comment
Hide comment
@npinto

npinto Mar 7, 2012

Contributor

Fixed / rebased.

Contributor

npinto commented Mar 7, 2012

Fixed / rebased.

@rgommers

This comment has been minimized.

Show comment
Hide comment
@rgommers

rgommers Mar 10, 2012

Member

This fails on OS X:

gcc-4.0 -arch ppc -arch i386 -isysroot /Developer/SDKs/MacOSX10.4u.sdk -g -bundle -undefined dynamic_lookup build/temp.macosx-10.3-fat-2.6/numpy/core/blasdot/_dotblas.o -Lbuild/temp.macosx-10.3-fat-2.6 -o build/lib.macosx-10.3-fat-2.6/numpy/core/_dotblas.so -Wl, -framework -Wl, Accelerate
i686-apple-darwin10-gcc-4.0.1: Accelerate: No such file or directory
powerpc-apple-darwin10-gcc-4.0.1: Accelerate: No such file or directory
lipo: can't figure out the architecture type of: /var/folders/Uu/UuXfo1NLFae4yyYpsCz-XE+++TI/-Tmp-//ccWfS3yr.out
i686-apple-darwin10-gcc-4.0.1: Accelerate: No such file or directory
powerpc-apple-darwin10-gcc-4.0.1: Accelerate: No such file or directory
lipo: can't figure out the architecture type of: /var/folders/Uu/UuXfo1NLFae4yyYpsCz-XE+++TI/-Tmp-//ccWfS3yr.out
error: Command "gcc-4.0 -arch ppc -arch i386 -isysroot /Developer/SDKs/MacOSX10.4u.sdk -g -bundle -undefined dynamic_lookup build/temp.macosx-10.3-fat-2.6/numpy/core/blasdot/_dotblas.o -Lbuild/temp.macosx-10.3-fat-2.6 -o build/lib.macosx-10.3-fat-2.6/numpy/core/_dotblas.so -Wl, -framework -Wl, Accelerate" failed with exit status 1

That's with python setup.py install, Python 2.6, OS X 10.6. Looks like a simple mistake somewhere, but I don't have time to debug right now.

Member

rgommers commented Mar 10, 2012

This fails on OS X:

gcc-4.0 -arch ppc -arch i386 -isysroot /Developer/SDKs/MacOSX10.4u.sdk -g -bundle -undefined dynamic_lookup build/temp.macosx-10.3-fat-2.6/numpy/core/blasdot/_dotblas.o -Lbuild/temp.macosx-10.3-fat-2.6 -o build/lib.macosx-10.3-fat-2.6/numpy/core/_dotblas.so -Wl, -framework -Wl, Accelerate
i686-apple-darwin10-gcc-4.0.1: Accelerate: No such file or directory
powerpc-apple-darwin10-gcc-4.0.1: Accelerate: No such file or directory
lipo: can't figure out the architecture type of: /var/folders/Uu/UuXfo1NLFae4yyYpsCz-XE+++TI/-Tmp-//ccWfS3yr.out
i686-apple-darwin10-gcc-4.0.1: Accelerate: No such file or directory
powerpc-apple-darwin10-gcc-4.0.1: Accelerate: No such file or directory
lipo: can't figure out the architecture type of: /var/folders/Uu/UuXfo1NLFae4yyYpsCz-XE+++TI/-Tmp-//ccWfS3yr.out
error: Command "gcc-4.0 -arch ppc -arch i386 -isysroot /Developer/SDKs/MacOSX10.4u.sdk -g -bundle -undefined dynamic_lookup build/temp.macosx-10.3-fat-2.6/numpy/core/blasdot/_dotblas.o -Lbuild/temp.macosx-10.3-fat-2.6 -o build/lib.macosx-10.3-fat-2.6/numpy/core/_dotblas.so -Wl, -framework -Wl, Accelerate" failed with exit status 1

That's with python setup.py install, Python 2.6, OS X 10.6. Looks like a simple mistake somewhere, but I don't have time to debug right now.

@npinto

This comment has been minimized.

Show comment
Hide comment
@npinto

npinto Mar 10, 2012

Contributor

Fixed. It should work now (at least it does on my OSX 10.6 w/ Python 2.7.2).

Contributor

npinto commented Mar 10, 2012

Fixed. It should work now (at least it does on my OSX 10.6 w/ Python 2.7.2).

@rgommers

This comment has been minimized.

Show comment
Hide comment
@rgommers

rgommers Mar 10, 2012

Member

That works now. I've tested on OS X 10.6 with Python 2.4 and 3.2, plus with numscons on 2.6. Also on Windows with Python 2.5. I don't use a site.cfg file though.

Could you perhaps ask on the mailing list if there's anyone who'd want to help test this? Preferably users of MKL / Umfpack / ...

Member

rgommers commented Mar 10, 2012

That works now. I've tested on OS X 10.6 with Python 2.4 and 3.2, plus with numscons on 2.6. Also on Windows with Python 2.5. I don't use a site.cfg file though.

Could you perhaps ask on the mailing list if there's anyone who'd want to help test this? Preferably users of MKL / Umfpack / ...

@npinto

This comment has been minimized.

Show comment
Hide comment
@npinto

npinto Mar 10, 2012

Contributor

We can test MKL (again ;-) and UMFPACK here with @poilvert. Anything else you would like to test?

Contributor

npinto commented Mar 10, 2012

We can test MKL (again ;-) and UMFPACK here with @poilvert. Anything else you would like to test?

@rgommers

This comment has been minimized.

Show comment
Hide comment
@rgommers

rgommers Mar 10, 2012

Member

No, that should be enough.

@cournape: you weren't sure about this change. It looks fine to me, and not all that invasive of a change. A number of people find this useful. Please comment if you still have some reservations.

Member

rgommers commented Mar 10, 2012

No, that should be enough.

@cournape: you weren't sure about this change. It looks fine to me, and not all that invasive of a change. A number of people find this useful. Please comment if you still have some reservations.

@npinto

This comment has been minimized.

Show comment
Hide comment
@npinto

npinto Mar 10, 2012

Contributor

I tried Intel MKL (from Composer XE 12.1) again on Gentoo with Python 2.7.2 and it works.

Contributor

npinto commented Mar 10, 2012

I tried Intel MKL (from Composer XE 12.1) again on Gentoo with Python 2.7.2 and it works.

@npinto

This comment has been minimized.

Show comment
Hide comment
@npinto

npinto Mar 11, 2012

Contributor

I also confirm that it works on Python 3.2.2 (Gentoo).

Contributor

npinto commented Mar 11, 2012

I also confirm that it works on Python 3.2.2 (Gentoo).

@cournape

This comment has been minimized.

Show comment
Hide comment
@cournape

cournape Mar 11, 2012

Member

On Sat, Mar 10, 2012 at 3:34 PM, Ralf Gommers <
reply@reply.github.com

wrote:

No, that should be enough.

@cournape: you weren't sure about this change. It looks fine to me, and
not all that invasive of a change. A number of people find this useful.
Please comment if you still have some reservations.

I still don't like it because it will be hard to support later with other
systems, but OTOH, it solves practical issues, so let's get it in,

David

Member

cournape commented Mar 11, 2012

On Sat, Mar 10, 2012 at 3:34 PM, Ralf Gommers <
reply@reply.github.com

wrote:

No, that should be enough.

@cournape: you weren't sure about this change. It looks fine to me, and
not all that invasive of a change. A number of people find this useful.
Please comment if you still have some reservations.

I still don't like it because it will be hard to support later with other
systems, but OTOH, it solves practical issues, so let's get it in,

David

@npinto

This comment has been minimized.

Show comment
Hide comment
@npinto

npinto Mar 11, 2012

Contributor

@rgommers, what tests should I run to insure that umfpack is working correctly? Do I need to install/test scipy as well?

Contributor

npinto commented Mar 11, 2012

@rgommers, what tests should I run to insure that umfpack is working correctly? Do I need to install/test scipy as well?

@npinto

This comment has been minimized.

Show comment
Hide comment
@npinto

npinto Mar 11, 2012

Contributor

Update: umfpack support compiles correctly on 2.7.2

Contributor

npinto commented Mar 11, 2012

Update: umfpack support compiles correctly on 2.7.2

rgommers added a commit that referenced this pull request Mar 14, 2012

Merge pull request #229 from npinto/system_info_lib_dirs.
Closes ticket 993.  Review: #229

There's some concern about future maintainability (by David C.), but overall
the benefits seem to outweigh the possible drawbacks.
@rgommers

This comment has been minimized.

Show comment
Hide comment
@rgommers

rgommers Mar 14, 2012

Member

OK, merged in commit a4dbfc1. Thanks Nicolas!

Member

rgommers commented Mar 14, 2012

OK, merged in commit a4dbfc1. Thanks Nicolas!

@rgommers rgommers closed this Mar 14, 2012

@npinto

This comment has been minimized.

Show comment
Hide comment
@npinto

npinto Mar 14, 2012

Contributor

Awesome. You're welcome. I'll contribute more, if time allows ;-)

Contributor

npinto commented Mar 14, 2012

Awesome. You're welcome. I'll contribute more, if time allows ;-)

@rgommers

This comment has been minimized.

Show comment
Hide comment
@rgommers

rgommers Mar 14, 2012

Member

More contributions would be great.

Member

rgommers commented Mar 14, 2012

More contributions would be great.

@npinto

This comment has been minimized.

Show comment
Hide comment
@npinto

npinto Mar 14, 2012

Contributor

No problem, I'm working on Ticket #2073. I'll get you a PR to review soon.

Contributor

npinto commented Mar 14, 2012

No problem, I'm working on Ticket #2073. I'll get you a PR to review soon.

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