Opened 2 years ago

Closed 2 years ago

#12423 closed defect (fixed)

Numpy wrongly assumes that any non-i386 OS X system is PowerPC

Reported by: jdemeyer Owned by: tbd
Priority: major Milestone: sage-5.0
Component: packages: standard Keywords:
Cc: Merged in: sage-5.0.beta5
Authors: Jeroen Demeyer Reviewers: William Stein
Report Upstream: Fixed upstream, but not in a stable release. Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

Apparently Numpy forgot about the existence of "x86_64" systems:

Python 2.7.2 (default, Feb  3 2012, 15:29:15)
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from distutils.util import get_platform
>>> get_platform()
'macosx-10.6-x86_64'

The bug is in src/numpy/distutils/system_info.py:

        if sys.platform=='darwin' and not os.environ.get('ATLAS',None):
            args = []
            link_args = []
            if get_platform()[-4:] == 'i386':
                intel = 1
            else:
                intel = 0
            if os.path.exists('/System/Library/Frameworks/Accelerate.framework/'):
                if intel:
                    args.extend(['-msse3'])
                else:
                    args.extend(['-faltivec'])
                link_args.extend(['-Wl,-framework','-Wl,Accelerate'])

This is already fixed upstream, see http://projects.scipy.org/numpy/ticket/2039. We apply the upstream fix.

spkg: http://boxen.math.washington.edu/home/jdemeyer/spkg/numpy-1.5.1.p1.spkg

Attachments (1)

numpy-1.5.1.p1.diff (5.9 KB) - added by jdemeyer 2 years ago.
Diff for the numpy spkg, for review only

Download all attachments as: .zip

Change History (15)

comment:1 Changed 2 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from Numpy wrongly assume that any non-i386 OS X system is PowerPC to Numpy wrongly assumes that any non-i386 OS X system is PowerPC

comment:2 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:4 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:5 Changed 2 years ago by jdemeyer

  • Description modified (diff)
  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. Little or no feedback.

comment:6 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:7 Changed 2 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Status changed from new to needs_review

comment:8 Changed 2 years ago by jdemeyer

  • Report Upstream changed from Reported upstream. Little or no feedback. to Reported upstream. Developers acknowledge bug.

comment:9 Changed 2 years ago by jdemeyer

  • Report Upstream changed from Reported upstream. Developers acknowledge bug. to Fixed upstream, but not in a stable release.
  • Status changed from needs_review to needs_work

comment:10 Changed 2 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Changed 2 years ago by jdemeyer

Diff for the numpy spkg, for review only

comment:11 follow-up: Changed 2 years ago by was

  • Status changed from needs_review to needs_work

Please remove everything from the patch that involves deleting support for Cygwin. Though I'm now not gung ho about the future of porting Sage to Cygwin and using it, I definitely don't think we should start silently sneaking in removing bits of cygwin support left and right all over in Sage! If somebody already did the work to figure out how to get numpy to build on Cygwin, let's not just delete that. What if I'm wrong, and it turns out that some genius does get Sage working well on Cygwin (and if nothing else, deleting cygwin support should be a different ticket).

The actual change for OS X 10.7 seems to be:

  • general clean up SPKG.txt -- good!
  • test for "intel" or "i386" instead of just "i386" -- can't be bad.

So I think the cygwin deletions should be deleted from this patch, then this should get a positive review once somebody checks that it actually works.

William

comment:12 in reply to: ↑ 11 ; follow-up: Changed 2 years ago by jdemeyer

  • Reviewers set to William Stein
  • Status changed from needs_work to positive_review

Replying to was:

Please remove everything from the patch that involves deleting support for Cygwin.

There is no such thing. I am only removing the patched file, not the patch. This is because I changed the spkg to use "patch" instead of "cp" to apply patches. So it should still work as before on Cygwin (even though I haven't tested this).

The actual change for OS X 10.7 seems to be:

  • general clean up SPKG.txt -- good!
  • test for "intel" or "i386" instead of just "i386" -- can't be bad.

So I think the cygwin deletions should be deleted from this patch, then this should get a positive review once somebody checks that it actually works.

Since there is no problem with Cygwin and since it does actually work, I take this as "positive_review".

comment:13 in reply to: ↑ 12 Changed 2 years ago by was

Replying to jdemeyer:

Replying to was:

Please remove everything from the patch that involves deleting support for Cygwin.

There is no such thing. I am only removing the patched file, not the patch. This is because I changed the spkg to use "patch" instead of "cp" to apply patches. So it should still work as before on Cygwin (even though I haven't tested this).

Thanks for the clarification. In fact, what you've done is really, really good.

The actual change for OS X 10.7 seems to be:

  • general clean up SPKG.txt -- good!
  • test for "intel" or "i386" instead of just "i386" -- can't be bad.

So I think the cygwin deletions should be deleted from this patch, then this should get a positive review once somebody checks that it actually works.

Since there is no problem with Cygwin and since it does actually work, I take this as "positive_review".

Indeed. OK, positive_review.

comment:14 Changed 2 years ago by jdemeyer

  • Merged in set to sage-5.0.beta5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.