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

[Backport release-24.11] arpack: add ISO C binding and top arpack-mpi attribute #367463

Merged
merged 6 commits into from
Jan 6, 2025

Conversation

nix-backports[bot]
Copy link

@nix-backports nix-backports bot commented Dec 22, 2024

Bot-based backport to release-24.11, triggered by a label in #357259.

  • Before merging, ensure that this backport is acceptable for the release.
    • Even as a non-commiter, if you find that it is not acceptable, leave a comment.

@paparodeo
Copy link
Contributor

don't backport this yet -- it breaks darwin.
https://hydra.nixos.org/build/282505341

it seems to add single precision api which Accelerate doesn't like so darwin needs to link with openblas.

@paparodeo
Copy link
Contributor

don't backport this yet -- it breaks darwin. https://hydra.nixos.org/build/282505341

it seems to add single precision api which Accelerate doesn't like so darwin needs to link with openblas.

will need to backport this PR #367594 for darwin

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Dec 23, 2024
@ofborg ofborg bot requested review from ttuegel and dotlambda December 23, 2024 10:58
Change 07cdea2 uncovered a bug in how
we build arpack on darwin with the Accelerate. Disable the Accelerate
framework and use openblas the default nixpkgs blas/lapack
implementation.

(cherry picked from commit 592da03)
use ninja, cmakeBool and remove unneeded install_name_tool usage in
postFixup as the library name is already properly set by the build

(cherry picked from commit 00baa26)
The arpack package included the eigen library and added it to the build
inputs but neglected to enable its use by setting `EIGEN=ON` in the
build flags.  Enable support for eigenvalue-problems solver based on ICB
and eigen and disable parallel checking as the tests fail when run in
parallel.

(cherry picked from commit 89e9ea2)
Copy darwin flags `-ff2c -fno-second-underscore` from workflow to use
Accelerate without segfaulting / failing tests and enable the Accelerate
framework when `useAccel` is true.
https://github.com/opencollab/arpack-ng/blob/804fa3149a0f773064198a8e883bd021832157ca/.github/workflows/jobs.yml#L184-L192

(cherry picked from commit 2f215b7)
@wolfgangwalther
Copy link
Contributor

will need to backport this PR #367594 for darwin

Cherry-picked the 4 commits from there in here as well.

@wolfgangwalther
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 367463


x86_64-linux

⏩ 7 packages marked as broken and skipped:
  • octavePackages.fem-fenics
  • octavePackages.level-set
  • octavePackages.parallel
  • octavePackages.sparsersb
  • octavePackages.tisean
  • octavePackages.vibes
  • octavePackages.vrml
❌ 1 package failed to build:
  • octavePackages.fits
✅ 125 packages built:
  • LPCNet
  • arpack
  • arpack-mpi
  • calculix
  • checkov
  • checkov.dist
  • gama
  • hal-hardware-analyzer
  • igraph
  • igraph.dev
  • igraph.doc
  • jupyter-all
  • libleidenalg
  • librsb
  • minc_widgets
  • octave
  • octaveFull
  • octavePackages.arduino
  • octavePackages.audio
  • octavePackages.bim
  • octavePackages.bsltl
  • octavePackages.cgi
  • octavePackages.communications
  • octavePackages.control
  • octavePackages.data-smoothing
  • octavePackages.database
  • octavePackages.dataframe
  • octavePackages.dicom
  • octavePackages.divand
  • octavePackages.doctest
  • octavePackages.econometrics
  • octavePackages.financial
  • octavePackages.fpl
  • octavePackages.fuzzy-logic-toolkit
  • octavePackages.ga
  • octavePackages.general
  • octavePackages.generate_html
  • octavePackages.geometry
  • octavePackages.gsl
  • octavePackages.image
  • octavePackages.image-acquisition
  • octavePackages.instrument-control
  • octavePackages.interval
  • octavePackages.io
  • octavePackages.linear-algebra
  • octavePackages.lssa
  • octavePackages.ltfat
  • octavePackages.mapping
  • octavePackages.matgeom
  • octavePackages.miscellaneous
  • octavePackages.msh
  • octavePackages.mvn
  • octavePackages.nan
  • octavePackages.ncarray
  • octavePackages.netcdf
  • octavePackages.nurbs
  • octavePackages.ocl
  • octavePackages.octclip
  • octavePackages.octproj
  • octavePackages.optics
  • octavePackages.optim
  • octavePackages.optiminterp
  • octavePackages.quaternion
  • octavePackages.queueing
  • octavePackages.signal
  • octavePackages.sockets
  • octavePackages.splines
  • octavePackages.statistics
  • octavePackages.stk
  • octavePackages.strings
  • octavePackages.struct
  • octavePackages.symbolic
  • octavePackages.tsa
  • octavePackages.video
  • octavePackages.windows
  • octavePackages.zeromq
  • octopus
  • openems
  • python311Packages.explorerscript
  • python311Packages.explorerscript.dist
  • python311Packages.igraph
  • python311Packages.igraph.dist
  • python311Packages.kmapper
  • python311Packages.kmapper.dist
  • python311Packages.leidenalg
  • python311Packages.leidenalg.dist
  • python311Packages.python-csxcad
  • python311Packages.python-csxcad.dist
  • python311Packages.python-openems
  • python311Packages.python-openems.dist
  • python311Packages.scikit-tda
  • python311Packages.scikit-tda.dist
  • python311Packages.skytemple-dtef
  • python311Packages.skytemple-dtef.dist
  • python311Packages.skytemple-files
  • python311Packages.skytemple-files.dist
  • python311Packages.skytemple-ssb-debugger
  • python311Packages.skytemple-ssb-debugger.dist
  • python311Packages.textnets
  • python311Packages.textnets.dist
  • python312Packages.explorerscript
  • python312Packages.explorerscript.dist
  • python312Packages.igraph
  • python312Packages.igraph.dist
  • python312Packages.kmapper
  • python312Packages.kmapper.dist
  • python312Packages.leidenalg
  • python312Packages.leidenalg.dist
  • python312Packages.python-csxcad
  • python312Packages.python-csxcad.dist
  • python312Packages.python-openems
  • python312Packages.python-openems.dist
  • python312Packages.scikit-tda
  • python312Packages.scikit-tda.dist
  • python312Packages.skytemple-dtef
  • python312Packages.skytemple-dtef.dist
  • python312Packages.skytemple-files
  • python312Packages.skytemple-files.dist
  • python312Packages.skytemple-ssb-debugger
  • python312Packages.skytemple-ssb-debugger.dist
  • python312Packages.textnets
  • python312Packages.textnets.dist
  • skytemple
  • skytemple.dist
  • vpv

@wolfgangwalther
Copy link
Contributor

aarch64-darwin build fails with:

       > FAILED: PARPACK/TESTS/MPI/icb_parpack_cpp
       > : && /nix/store/l8n4jk7l60yb7jag3i17bz57s7rvf0rd-clang-wrapper-16.0.6/bin/clang++ -O3 -DNDEBUG -isysroot /nix/store/bbwmxj5wv6nh3cydiyijp80zn30q5svf-apple-sdk-11.3/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk -Wl,-search_paths_first -Wl,-headerpad_max_install_names -Wl,-flat_namespace CMakeFiles/icb_parpack_cpp.dir/PARPACK/TESTS/MPI/icb_parpack_cpp.cpp.o -o PARPACK/TESTS/MPI/icb_parpack_cpp  -Wl,-rpath,/tmp/nix-build-arpack-3.9.1.drv-1/source/build/lib  lib/libparpack.2.1.0.dylib  lib/libarpack.2.1.0.dylib  /nix/store/r6fzlf171lmmkrpc5685nd448366g2vx-blas-3/lib/libblas.dylib  /nix/store/6fp60l0lpzadh59qwy44xlbgxkwwkskx-lapack-3/lib/liblapack.dylib  /nix/store/1jlvrhcqq9sv5n2bngvnc11bg8gp2rpb-openmpi-5.0.6/lib/libmpi_usempif08.dylib  /nix/store/1jlvrhcqq9sv5n2bngvnc11bg8gp2rpb-openmpi-5.0.6/lib/libmpi_usempi_ignore_tkr.dylib  /nix/store/1jlvrhcqq9sv5n2bngvnc11bg8gp2rpb-openmpi-5.0.6/lib/libmpi_mpifh.dylib  /nix/store/1jlvrhcqq9sv5n2bngvnc11bg8gp2rpb-openmpi-5.0.6/lib/libmpi.dylib && :
       > ld: file not found: @rpath/libquadmath.0.dylib for architecture arm64
       > clang-16: error: linker command failed with exit code 1 (use -v to see invocation)
       > ninja: build stopped: subcommand failed.

@wolfgangwalther wolfgangwalther marked this pull request as draft January 6, 2025 08:53
@paparodeo
Copy link
Contributor

paparodeo commented Jan 6, 2025

aarch64-darwin build fails with:

       > FAILED: PARPACK/TESTS/MPI/icb_parpack_cpp
       > : && /nix/store/l8n4jk7l60yb7jag3i17bz57s7rvf0rd-clang-wrapper-16.0.6/bin/clang++ -O3 -DNDEBUG -isysroot /nix/store/bbwmxj5wv6nh3cydiyijp80zn30q5svf-apple-sdk-11.3/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk -Wl,-search_paths_first -Wl,-headerpad_max_install_names -Wl,-flat_namespace CMakeFiles/icb_parpack_cpp.dir/PARPACK/TESTS/MPI/icb_parpack_cpp.cpp.o -o PARPACK/TESTS/MPI/icb_parpack_cpp  -Wl,-rpath,/tmp/nix-build-arpack-3.9.1.drv-1/source/build/lib  lib/libparpack.2.1.0.dylib  lib/libarpack.2.1.0.dylib  /nix/store/r6fzlf171lmmkrpc5685nd448366g2vx-blas-3/lib/libblas.dylib  /nix/store/6fp60l0lpzadh59qwy44xlbgxkwwkskx-lapack-3/lib/liblapack.dylib  /nix/store/1jlvrhcqq9sv5n2bngvnc11bg8gp2rpb-openmpi-5.0.6/lib/libmpi_usempif08.dylib  /nix/store/1jlvrhcqq9sv5n2bngvnc11bg8gp2rpb-openmpi-5.0.6/lib/libmpi_usempi_ignore_tkr.dylib  /nix/store/1jlvrhcqq9sv5n2bngvnc11bg8gp2rpb-openmpi-5.0.6/lib/libmpi_mpifh.dylib  /nix/store/1jlvrhcqq9sv5n2bngvnc11bg8gp2rpb-openmpi-5.0.6/lib/libmpi.dylib && :
       > ld: file not found: @rpath/libquadmath.0.dylib for architecture arm64
       > clang-16: error: linker command failed with exit code 1 (use -v to see invocation)
       > ninja: build stopped: subcommand failed.

womp womp. linking with flat_namespace exposes a bug somewhere in our stack. this would fix it #370526 as would removing -flat_namespace or linking with Accelerate instead of openblas which pulls in liibgfortran which has the @rpath that causes the error libgfortran is still getting pulled in so what blas library is irrelevant-- tho linking with Accelerate and then having openblas pulled in via another dependency + flat_namespace = symbol conflicts.

or adding -L${lib.getlib gfotran.cc} to the LDFLAGS I think.

@paparodeo
Copy link
Contributor

paparodeo commented Jan 6, 2025

oh -- this is only the arpack-mpi build that breaks -- that is broken already. there just wasn't an attribute for it. that shouldn't stop this from back porting. it doesn't break anything new and it fixes bugs with Accelerate.

@wolfgangwalther
Copy link
Contributor

oh -- this is only the arpack-mpi build that breaks -- that is broken already. there just wasn't an attribute for it. that shouldn't stop this from back porting. it doesn't break anything new and it fixes bugs with Accelerate.

Ah, mh. Maybe that package should be renamed then, because I got the error as something like this:

For full logs, run 'nix log /nix/store/43ardz2a2xhlz2f3zknainqvq2wx3bz2-arpack-3.9.1.drv'.

@wolfgangwalther
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 367463


aarch64-darwin

⏩ 24 packages marked as broken and skipped:
  • librsb
  • octavePackages.fem-fenics
  • octavePackages.gsl
  • octavePackages.level-set
  • octavePackages.ocl
  • octavePackages.parallel
  • octavePackages.sparsersb
  • octavePackages.strings
  • octavePackages.tisean
  • octavePackages.vibes
  • octavePackages.vrml
  • python311Packages.skytemple-dtef
  • python311Packages.skytemple-dtef.dist
  • python311Packages.skytemple-files
  • python311Packages.skytemple-files.dist
  • python311Packages.textnets
  • python311Packages.textnets.dist
  • python312Packages.skytemple-dtef
  • python312Packages.skytemple-dtef.dist
  • python312Packages.skytemple-files
  • python312Packages.skytemple-files.dist
  • python312Packages.textnets
  • python312Packages.textnets.dist
  • vpv
❌ 5 packages failed to build:
  • arpack-mpi
  • octavePackages.fits
  • octavePackages.geometry
  • octavePackages.ltfat
  • octavePackages.mapping
✅ 85 packages built:
  • LPCNet
  • arpack
  • calculix
  • checkov
  • checkov.dist
  • gama
  • hal-hardware-analyzer
  • igraph
  • igraph.dev
  • igraph.doc
  • jupyter-all
  • libleidenalg
  • minc_widgets
  • octave
  • octaveFull
  • octavePackages.bim
  • octavePackages.bsltl
  • octavePackages.cgi
  • octavePackages.communications
  • octavePackages.control
  • octavePackages.data-smoothing
  • octavePackages.database
  • octavePackages.dataframe
  • octavePackages.dicom
  • octavePackages.divand
  • octavePackages.doctest
  • octavePackages.econometrics
  • octavePackages.financial
  • octavePackages.fpl
  • octavePackages.fuzzy-logic-toolkit
  • octavePackages.ga
  • octavePackages.general
  • octavePackages.generate_html
  • octavePackages.image
  • octavePackages.instrument-control
  • octavePackages.interval
  • octavePackages.io
  • octavePackages.linear-algebra
  • octavePackages.lssa
  • octavePackages.matgeom
  • octavePackages.miscellaneous
  • octavePackages.msh
  • octavePackages.mvn
  • octavePackages.nan
  • octavePackages.ncarray
  • octavePackages.netcdf
  • octavePackages.nurbs
  • octavePackages.octclip
  • octavePackages.octproj
  • octavePackages.optics
  • octavePackages.optim
  • octavePackages.optiminterp
  • octavePackages.quaternion
  • octavePackages.queueing
  • octavePackages.signal
  • octavePackages.sockets
  • octavePackages.splines
  • octavePackages.statistics
  • octavePackages.stk
  • octavePackages.struct
  • octavePackages.symbolic
  • octavePackages.tsa
  • octavePackages.video
  • octavePackages.windows
  • octavePackages.zeromq
  • python311Packages.explorerscript
  • python311Packages.explorerscript.dist
  • python311Packages.igraph
  • python311Packages.igraph.dist
  • python311Packages.kmapper
  • python311Packages.kmapper.dist
  • python311Packages.leidenalg
  • python311Packages.leidenalg.dist
  • python311Packages.scikit-tda
  • python311Packages.scikit-tda.dist
  • python312Packages.explorerscript
  • python312Packages.explorerscript.dist
  • python312Packages.igraph
  • python312Packages.igraph.dist
  • python312Packages.kmapper
  • python312Packages.kmapper.dist
  • python312Packages.leidenalg
  • python312Packages.leidenalg.dist
  • python312Packages.scikit-tda
  • python312Packages.scikit-tda.dist

@wolfgangwalther wolfgangwalther marked this pull request as ready for review January 6, 2025 09:42
@wolfgangwalther wolfgangwalther merged commit 0f73176 into release-24.11 Jan 6, 2025
22 checks passed
@wolfgangwalther wolfgangwalther deleted the backport-357259-to-release-24.11 branch January 6, 2025 09:44
@paparodeo
Copy link
Contributor

paparodeo commented Jan 6, 2025

oh -- this is only the arpack-mpi build that breaks -- that is broken already. there just wasn't an attribute for it. that shouldn't stop this from back porting. it doesn't break anything new and it fixes bugs with Accelerate.

Ah, mh. Maybe that package should be renamed then, because I got the error as something like this:

For full logs, run 'nix log /nix/store/43ardz2a2xhlz2f3zknainqvq2wx3bz2-arpack-3.9.1.drv'.

k. will fix the linker error too and send the PR to master. [edit] tho tests seem to be failing so maybe not.

@paparodeo
Copy link
Contributor

useMpi fix for for master: #371421 would require eeffd96 for tests to pass on 24.11

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

Successfully merging this pull request may close these issues.

3 participants