Opened 7 years ago

Closed 5 years ago

#37097 closed enhancement (fixed)

DSDP should include atlas variant *option*

Reported by: dallas.johnston@… Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version:
Keywords: haspatch Cc: jjstickel (Jonathan Stickel), petrrr, mbrethen
Port: DSDP

Description

Many people have run into issues building DSDP and other ports that depend on it (like cvxopt) due to linker issues with building against ATLAS. It is suggested that a atlas variant option is created for DSDP in order to enable default linking against the Accelerator Framework libs provided by Apple.

I have tested building cvxopt with -dsdp and it -atlas, which compiles and runs without errors.

Attachments (5)

Portfile (2.5 KB) - added by dallas.johnston@… 7 years ago.
Modified Portfile to implement this. Tested to build without error.
DSDP.diff (751 bytes) - added by ryandesign (Ryan Schmidt) 7 years ago.
dallas.johnston's proposed changes
dsdp_Portfile.diff (1.4 KB) - added by jjstickel (Jonathan Stickel) 5 years ago.
patch-make.include.diff.diff (730 bytes) - added by jjstickel (Jonathan Stickel) 5 years ago.
diff of the patch
dsdp_Portfile.2.diff (1.2 KB) - added by mbrethen 5 years ago.
rev not changed

Download all attachments as: .zip

Change History (22)

comment:1 Changed 7 years ago by ryandesign (Ryan Schmidt)

Keywords: dsdp atlas cvxopt removed
Owner: changed from macports-tickets@… to mnick@…
Priority: HighNormal
Type: defectenhancement
Version: 2.1.2

Changed 7 years ago by dallas.johnston@…

Attachment: Portfile added

Modified Portfile to implement this. Tested to build without error.

comment:2 Changed 7 years ago by jjstickel (Jonathan Stickel)

Cc: jjstickel@… added

Cc Me!

comment:3 Changed 7 years ago by jjstickel (Jonathan Stickel)

Also see #35851.

comment:4 Changed 7 years ago by ryandesign (Ryan Schmidt)

Keywords: haspatch added
Port: DSDP added; dsdp removed

In the future please attach a unified diff instead of a full portfile; this makes it easier to review your proposed changes. I'm attaching a diff of your changes now.

Your proposed atlas variant is insufficient: it only adds a dependency on atlas, without doing anything to ensure that if atlas is already installed it will not be used unless the atlas variant is selected.

Changed 7 years ago by ryandesign (Ryan Schmidt)

Attachment: DSDP.diff added

dallas.johnston's proposed changes

comment:5 Changed 7 years ago by ryandesign (Ryan Schmidt)

Also, in other ports that offer the choice of ATLAS or Apple's Accelerate framework—like octave—there are two conflicting variants: atlas and accelerate; perhaps we should continue that tradition here—assuming DSDP can be made to work with today's ATLAS (#35851); if it can't, there's no point keeping an ATLAS option at all.

comment:6 Changed 6 years ago by mf2k (Frank Schima)

Owner: changed from mnick@… to macports-tickets@…

mnick has retired.

comment:7 Changed 6 years ago by jjstickel (Jonathan Stickel)

Note that the atlas linking issue has now been resolved in #35851. Nonetheless, it may be worthwhile to make atlas a variant.

comment:8 Changed 6 years ago by petrrr

Cc: petr@… added

Cc Me!

Changed 5 years ago by jjstickel (Jonathan Stickel)

Attachment: dsdp_Portfile.diff added

Changed 5 years ago by jjstickel (Jonathan Stickel)

diff of the patch

comment:9 Changed 5 years ago by jjstickel (Jonathan Stickel)

The new patchfiles enable accelerate framework for LAPACK and BLAS by default (no variant selected) and provide an atlas variant. I have not tested the accelerate framework (-atlas), but the compile flags were reported to work by Mark Brethen (https://lists.macosforge.org/pipermail/macports-users/2014-July/036012.html).

comment:10 Changed 5 years ago by mbrethen

also see #44503

jjstickel: I'm curious why you placed the atlas variant inside the octave variant? Is this the only situation where you'd want/need atlas?

Last edited 5 years ago by mbrethen (previous) (diff)

comment:11 in reply to:  10 ; Changed 5 years ago by jjstickel (Jonathan Stickel)

Replying to mark.brethen@…:

also see #44503

jjstickel: I'm curious why you placed the atlas variant inside the octave variant? Is this the only situation where you'd want/need atlas?

It's not (the way trac shows the patch makes it look that way). It is its own variant, and it triggers some action in the post-patch block.

Anyway, please test the patches and report back. Thanks.

comment:12 Changed 5 years ago by ryandesign (Ryan Schmidt)

If you're just adding a variant, which is not going to be on by default, then there is no need to increase the port's revision.

comment:13 in reply to:  12 Changed 5 years ago by jjstickel (Jonathan Stickel)

Replying to ryandesign@…:

If you're just adding a variant, which is not going to be on by default, then there is no need to increase the port's revision.

I thought about whether a revision increment was necessary; I included it because the default behavior is changing from compiling with atlas to compiling with accelerate framework. But I guess forcing an upgrade on users wouldn't help them catch this change anyway.

comment:14 Changed 5 years ago by mbrethen

Cc: mark.brethen@… added

Cc Me!

comment:15 in reply to:  11 Changed 5 years ago by mbrethen

Replying to jjstickel@…:

Anyway, please test the patches and report back. Thanks.

Okay, I'll let you know.

Last edited 5 years ago by mbrethen (previous) (diff)

Changed 5 years ago by mbrethen

Attachment: dsdp_Portfile.2.diff added

rev not changed

comment:16 Changed 5 years ago by mbrethen

It works. I uploaded another portfile patch, removing the rev change (if that's really the case).

comment:17 Changed 5 years ago by mf2k (Frank Schima)

Resolution: fixed
Status: newclosed

Sorry for the delay. r127980.

Note: See TracTickets for help on using tickets.