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

scotch v7.0.2 + cmake build #70

Merged
merged 49 commits into from
Feb 1, 2023

Conversation

regro-cf-autotick-bot
Copy link
Contributor

It is very likely that the current package version for this feedstock is out of date.

Checklist before merging this PR:

  • Dependencies have been updated if changed: see upstream
  • Tests have passed
  • Updated license if changed and license_file is packaged

Information about this PR:

  1. Feel free to push to the bot's branch to update this PR if needed.
  2. The bot will almost always only open one PR per version.
  3. The bot will stop issuing PRs if more than 3 version bump PRs generated by the bot are open. If you don't want to package a particular version please close the PR.
  4. If you want these PRs to be merged automatically, make an issue with @conda-forge-admin,please add bot automerge in the title and merge the resulting PR. This command will add our bot automerge feature to your feedstock.
  5. If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

Pending Dependency Version Updates

Here is a list of all the pending dependency version updates for this repo. Please double check all dependencies before merging.

Name Upstream Version Current Version
scotch 7.0.1 Anaconda-Server Badge

Dependency Analysis

We couldn't run dependency analysis due to an internal error in the bot. :( Help is very welcome!

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/autotick-bot/actions/runs/1962647822, please use this URL for debugging.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@traversaro
Copy link
Contributor

From this version we may consider switching to the CMake build system. I did not tried, this would permit to create shared library natively by setting BUILD_SHARED_LIBS to ON , and dropping the complex patches on the Makefile. However, probably we need other patches for the cross-compilation aspects. Due to the run_exports (see https://github.com/conda-forge/scotch-feedstock/blob/master/recipe/meta.yaml#L28) and the probably ABI incompatibilities, if we do not this now problably we will need to wait for a new scotch release.

@minrk
Copy link
Member

minrk commented May 20, 2022

Started looking into this, but it appears shared libs don't actually work, even with the cmake builds: https://gitlab.inria.fr/scotch/scotch/-/issues/16

I tried, and linking fails because internal dependencies aren't correctly expressed (e.g. libscotch -> libscotcherr). Might not be a big patch, though.

@traversaro
Copy link
Contributor

The error:

CMake Error at CMakeLists.txt:44 (project):
  No CMAKE_CXX_COMPILER could be found.

  Tell CMake where to find the compiler by setting either the environment
  variable "CXX" or the CMake cache entry CMAKE_CXX_COMPILER to the full path
  to the compiler, or to the compiler name if it is in the PATH.


-- Configuring incomplete, errors occurred!

is because CMake by default requires both a C and a C++ compiler, I think this can be fixed by changing the beginning of CMakeLists.txt from:

project(SCOTCH)
cmake_minimum_required(VERSION 3.10)
enable_language(C Fortran)

to:

cmake_minimum_required(VERSION 3.10)
project(SCOTCH LANGUAGES C Fortran) 

patch upstream to use cmake COMPONENTs to manage the split install

separate outputs for scotchmetis/parmetis
@minrk
Copy link
Member

minrk commented May 20, 2022

linking wasn't actually an issue, just needed -shared because of the relationship of libscotch to libscotcherr.

I think the main question now is outputs, which are easier to control with e.g. cmake components. Do we want to stick with the current two outputs (scotch: libscotch, scotch binaries, libesmumps, libscotchmetis, ptscotch: ptscotch bins, libptscotch, libptscotchparmetis (no libptesmumps anymore). Or split further? The scotch cmake files are organized along scotch, libscotch, libesmumps, libmetis (+ pt variants).

I looked at ubuntu, and they have:

  • lib[pt]scotch (all libs)
  • [pt]scotch (bins, man)

What about the metis/parmets.h headers? Previously, we moved them into $PREFIX/include/scotch to avoid conflicts with 'true' metis/parmetis.

Other choices:

  • keep our scotch prefix
  • follow debian, and use include/[par]metis/ prefix (seems weird to
  • leave path alone (conflicts with metis/parmetis), but split libscotchmetis output which is free to conflict
  • add scotchmetis/parmetis outputs and change path to one of the above choices

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • Failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

@regro-cf-autotick-bot regro-cf-autotick-bot mentioned this pull request Dec 24, 2022
3 tasks
@Tobias-Fischer Tobias-Fischer mentioned this pull request Jan 2, 2023
5 tasks
@Tobias-Fischer Tobias-Fischer changed the title scotch v7.0.1 scotch v7.0.2 + cmake build Jan 4, 2023
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@Tobias-Fischer
Copy link
Contributor

Hi @traversaro @minrk, I pushed this PR a bit and it now passes for all platforms except osx-arm64. On osx-arm64, we will need to build osx-64 first to then use the resulting binaries. Would either of you happy to take a look at that?

@Tobias-Fischer
Copy link
Contributor

So my current idea is to create a new basic cmake project that generates the dummysizes executable, like so:

cmake_minimum_required(VERSION 3.10)
project(dummysizes LANGUAGES C)

set(GENERATED_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include)
set(LIBSCOTCH_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/libscotch)
file(MAKE_DIRECTORY ${GENERATED_INCLUDE_DIR})

option(BUILD_PTSCOTCH "Build PT-Scotch" ON)

add_executable(dummysizes ../libscotch/dummysizes.c)
set_target_properties(dummysizes PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
if(BUILD_PTSCOTCH)
  find_package(MPI COMPONENTS C)
  if(NOT MPI_C_FOUND)
    message(FATAL_ERROR "MPI required to compile PT-Scotch")
  endif()
  add_executable(ptdummysizes ../libscotch/dummysizes.c)
  set_target_properties(ptdummysizes PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
  target_link_libraries(ptdummysizes PRIVATE MPI::MPI_C)
  add_dependencies(ptdummysizes scotch_h)
  set_target_properties(ptdummysizes PROPERTIES
    COMPILE_FLAGS -DSCOTCH_PTSCOTCH)
endif(BUILD_PTSCOTCH)

##############
#  scotch.h  #
##############

# Generate scotch.h
add_custom_command(OUTPUT ${GENERATED_INCLUDE_DIR}/scotch.h
  COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/../libscotch/library.h ${CMAKE_CURRENT_BINARY_DIR}
  COMMAND $<TARGET_FILE:dummysizes> ${CMAKE_CURRENT_BINARY_DIR}/library.h ${GENERATED_INCLUDE_DIR}/scotch.h DEPENDS dummysizes
  COMMENT "Generate scotch.h")
add_custom_target(scotch_h
  DEPENDS "${GENERATED_INCLUDE_DIR}/scotch.h")

This should be invoked by the native compiler. Then the actual cmake files would need to be adapted to use the generated executables rather than $<TARGET_FILE:dummysizes>. Any thoughts?

@minrk
Copy link
Member

minrk commented Jan 30, 2023

Re the hashes: it’s probably easier to just add mpi is dependency, even if not strictly needed?

Yes, adding - {{ mpi }} to host deps works, too (you'll need it in both scotch and ptscotch, and copy the ignore_run_exports from libscotch to scotch, too). That's in fact what I've always done myself, I just learned about build.force_use_keys today in trying to find a cleaner solution to this same problem. Whatever you prefer is fine!

The upstream issue is conda/conda-build#3308 btw.

@Tobias-Fischer
Copy link
Contributor

Re the hashes: it’s probably easier to just add mpi is dependency, even if not strictly needed?

Yes, adding - {{ mpi }} to host deps works, too (you'll need it in both scotch and ptscotch, and copy the ignore_run_exports from libscotch to scotch, too). That's in fact what I've always done myself, I just learned about build.force_use_keys today in trying to find a cleaner solution to this same problem. Whatever you prefer is fine!

The upstream issue is conda/conda-build#3308 btw.

All done, thanks :)

@Tobias-Fischer
Copy link
Contributor

Thanks again for the reviews @minrk @traversaro - all your comments have now been addressed :)

@minrk
Copy link
Member

minrk commented Jan 31, 2023

Looks like conda-forge had a quota issue on Travis, but all looks well. Presumably this will reset at the start of the month tomorrow, and we can re-run and merge.

travis is having problems
@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict.
Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@minrk
Copy link
Member

minrk commented Feb 1, 2023

month turnover didn't fix travis, so I switched those builds over to azure, which is a lot slower but tends to be more reliable. Since cross-compile is working for mac-arm, it should hopefully work for these, too, without any further changes.

@dalcinl
Copy link
Contributor

dalcinl commented Feb 1, 2023

@minrk By now, Travis-CI is almost dead for open-source projects, conda-forge should stop relying on it.

@minrk
Copy link
Member

minrk commented Feb 1, 2023

Yup, working on it. There are no other ppc providers that I'm aware of.

@minrk minrk added the automerge Merge the PR when CI passes label Feb 1, 2023
@github-actions github-actions bot merged commit c0f228c into conda-forge:main Feb 1, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 1, 2023

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

  • linter: passed
  • azure: passed

Thus the PR was passing and merged! Have a great day!

@regro-cf-autotick-bot regro-cf-autotick-bot deleted the 7.0.1_h02367e branch February 1, 2023 11:58
@minrk
Copy link
Member

minrk commented Feb 1, 2023

Huzzah, thanks @Tobias-Fischer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge the PR when CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants