Skip to content

Up1600#52

Closed
RaulPPelaez wants to merge 37 commits into
conda-forge:mainfrom
RaulPPelaez:up1600
Closed

Up1600#52
RaulPPelaez wants to merge 37 commits into
conda-forge:mainfrom
RaulPPelaez:up1600

Conversation

@RaulPPelaez
Copy link
Copy Markdown

@RaulPPelaez RaulPPelaez commented Feb 3, 2025

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Closes #42

@conda-forge-admin
Copy link
Copy Markdown
Contributor

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/meta.yaml) and found it was in an excellent condition.

@conda-forge-admin
Copy link
Copy Markdown
Contributor

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/meta.yaml) and found some lint.

Here's what I've got...

For recipe/meta.yaml:

  • ❌ You are setting c_stdlib_version below the current global baseline in conda-forge (10.13). If this is your intention, you also need to override MACOSX_DEPLOYMENT_TARGET (with the same value) locally.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/13112822110. Examine the logs at this URL for more detail.

@conda-forge-admin
Copy link
Copy Markdown
Contributor

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/meta.yaml) and found it was in an excellent condition.

@h-vetinari
Copy link
Copy Markdown
Member

Cool that you're reviving this feedstock - it could really use a maintainer! :)

I tried this a while ago, here are some relevant notes

@RaulPPelaez
Copy link
Copy Markdown
Author

Thanks @h-vetinari! needed it for a project and realised the version in cf is quite outdated. Lets see if I can crack it.
I see the discussion regarding pytrilinos, but AFAICT that feedstock is also abandoned.

@RaulPPelaez
Copy link
Copy Markdown
Author

Arggg almost there! but the OSX build is failing with some linker error right at the end:

[100%] Linking CXX shared library libpiro.dylib
Undefined symbols for architecture x86_64:
  "Teuchos::RCP<Tempus::IntegratorForwardSensitivity<double>> Tempus::createIntegratorForwardSensitivity<double>(Teuchos::RCP<Teuchos::ParameterList>, Teuchos::RCP<Thyra::ModelEvaluator<double>> const&, Teuchos::RCP<Thyra::ModelEvaluator<double>> const&, Teuchos::RCP<Thyra::ModelEvaluator<double>> const&)", referenced from:
      Teuchos::RCP<Tempus::IntegratorForwardSensitivity<double>> Tempus::createIntegratorForwardSensitivity<double>(Teuchos::RCP<Teuchos::ParameterList>, Teuchos::RCP<Thyra::ModelEvaluator<double>> const&) in Piro_PerformAnalysis.cpp.o
ld: symbol(s) not found for architecture x86_64
x86_64-apple-darwin13.4: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [packages/piro/src/CMakeFiles/piro.dir/build.make:304: packages/piro/src/libpiro.16.0.0.dylib] Error 1
make[1]: *** [CMakeFiles/Makefile2:12444: packages/piro/src/CMakeFiles/piro.dir/all] Error 2
make: *** [Makefile:166: all] Error 2

@RaulPPelaez
Copy link
Copy Markdown
Author

Ok @h-vetinari, I got it building. Feel free to add me as maintainer. Would be great if you give the PR a review :P
There are a lot of old migrations still open, does this PR supersede them?

@h-vetinari
Copy link
Copy Markdown
Member

Great news! 👏

Feel free to add me as maintainer.

Can you add yourself in this PR?

There are a lot of old migrations still open, does this PR supersede them?

Many of them, yes (because those have been closed in the meantime and a rerender from the global pinning does the same as they would have done then).

Would be great if you give the PR a review :P

let me have a look :)

Copy link
Copy Markdown
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks alright, just a few things that need cleaning up (+ a rerender)

Comment thread conda-forge.yml
Comment thread recipe/conda_build_config.yaml Outdated
@h-vetinari
Copy link
Copy Markdown
Member

h-vetinari commented Feb 10, 2025

Not sure what's wrong with the osx builds just now. I restarted, but it happened again in both jobs:

+ conda-build ./recipe -m ./.ci_support/osx_64_mpimpich.yaml --suppress-variables --clobber-file ./.ci_support/clobber_osx_64_mpimpich.yaml --extra-meta flow_run_id=azure_20250210.3.2 remote_url=*** sha=f41460d32cf75d8da45e5555aa2266cf96b0d3f9
Traceback (most recent call last):
  File "/Users/runner/miniforge3/bin/conda-build", line 7, in <module>
    from conda_build.cli.main_build import execute
  [...]
  File "/Users/runner/miniforge3/lib/python3.12/sqlite3/dbapi2.py", line 27, in <module>
    from _sqlite3 import *
ImportError: dlopen(/Users/runner/miniforge3/lib/python3.12/lib-dynload/_sqlite3.cpython-312-darwin.so, 0x0002): Symbol not found: _sqlite3_enable_load_extension
  Referenced from: <7B10479C-424F-347E-8F22-960E25E02A95> /Users/runner/miniforge3/lib/python3.12/lib-dynload/_sqlite3.cpython-312-darwin.so
  Expected in:     <94802878-251A-3347-83F3-16767EBFC7CC> /usr/lib/libsqlite3.dylib

It also happens before the recipe is actually run (and the rerender commit changed nothing of consequence on osx)


Beyond that, looking at the last passing run, there's a few more things that need fixing. For example, we end up shipping

?rwxrwxrwx 0/0          0 2025-02-07 12:37:38 lib/libgtest.so -> libgtest.so.16.0.0 
?rwxrwxrwx 0/0          0 2025-02-07 12:36:55 lib/libgtest.so.16 -> libgtest.so.16.0.0 
?rwxrwxr-x 0/0     485280 2025-02-07 12:37:41 lib/libgtest.so.16.0.0 

which should be solvable by adding gtest to the host environment.

We also end up reshipping kokkos, which likewise needs to go away:

?rwxrwxrwx 0/0          0 2025-02-07 12:37:38 lib/libkokkosalgorithms.so -> libkokkosalgorithms.so.16.0.0 
?rwxrwxrwx 0/0          0 2025-02-07 12:36:55 lib/libkokkosalgorithms.so.16 -> libkokkosalgorithms.so.16.0.0 
?rwxrwxr-x 0/0      15112 2025-02-07 12:37:39 lib/libkokkosalgorithms.so.16.0.0 
?rwxrwxrwx 0/0          0 2025-02-07 12:36:55 lib/libkokkoscontainers.so.16 -> libkokkoscontainers.so.16.0.0 
?rwxrwxr-x 0/0      16176 2025-02-07 12:37:43 lib/libkokkoscontainers.so.16.0.0 
?rwxrwxrwx 0/0          0 2025-02-07 12:36:56 lib/libkokkoscore.so.16 -> libkokkoscore.so.16.0.0 
?rwxrwxr-x 0/0     444400 2025-02-07 12:37:42 lib/libkokkoscore.so.16.0.0 
?rwxrwxrwx 0/0          0 2025-02-07 12:36:55 lib/libkokkoskernels.so.16 -> libkokkoskernels.so.16.0.0 
?rwxrwxr-x 0/0    7498352 2025-02-07 12:37:40 lib/libkokkoskernels.so.16.0.0 
?rwxrwxrwx 0/0          0 2025-02-07 12:36:55 lib/libkokkossimd.so.16 -> libkokkossimd.so.16.0.0 
?rwxrwxr-x 0/0      15104 2025-02-07 12:37:43 lib/libkokkossimd.so.16.0.0 
?rwxrwxrwx 0/0          0 2025-02-07 12:37:38 lib/libkokkostsqr.so -> libkokkostsqr.so.16.0.0 
?rwxrwxrwx 0/0          0 2025-02-07 12:36:56 lib/libkokkostsqr.so.16 -> libkokkostsqr.so.16.0.0 
?rwxrwxr-x 0/0     563920 2025-02-07 12:37:40 lib/libkokkostsqr.so.16.0.0

More importantly, it seems that trilinos sets its own version as the SOVERSION for all its vendored libraries (actual versions see here), so this would conflict and possibly break badly together with the actual kokkos.

@RaulPPelaez
Copy link
Copy Markdown
Author

Other feedstocks OSX builds are failing, I reckon this is some transient issue outside the scope of this feedstock...

@RaulPPelaez
Copy link
Copy Markdown
Author

The builds now fail with this message at config time:

CMake Error at packages/tpetra/CMakeLists.txt:47 (MESSAGE):
  Tpetra: Incompatible external Kokkos version 4.3.0.  Tpetra requires 4.3.1

It seems like Trilinos needs a very specific version of kokkos to compile. Alas, the required one is not on conda-forge:

kokkos                        4.3.00      h6772cb2_0  conda-forge
kokkos                        4.3.00      h69fb06d_0  conda-forge
kokkos                        4.3.00      hb7983d7_0  conda-forge
kokkos                        4.3.00      hd538f9c_1  conda-forge
kokkos                        4.4.01 cuda11h6b829ae_0  conda-forge
kokkos                        4.4.01 cuda12h3c2d641_0  conda-forge
kokkos                        4.4.01      h56b8afe_0  conda-forge

I do not know what the best course of action is here. It does not make sense for someone wanting kokkos to install the whole of trilinos. Kokkos is a part of trilinos, so if conda-forge shipping that separately, should this feedstock be split into many, one for each trilinos package?

@h-vetinari
Copy link
Copy Markdown
Member

I do not know what the best course of action is here.

The best course of action IMO is to patch out the too-tight constraint in tpetra and keep depending on our own kokkos here. Even more so because (due to kokkos' silly version scheme, where patch versions have a leading 0), there isn't actually a v4.3.1 of kokkos.

Kokkos is a part of trilinos, so if conda-forge shipping that separately, should this feedstock be split into many, one for each trilinos package?

One does not necessarily imply the other. Unvendoring kokkos (and potentially other pieces of trilinos that we might be shipping separately already) does not mean that the package has to be split into many components. It just means that trilinos should first and foremost make use of the existing packages, unless very strong reasons not to materialize.

@RaulPPelaez
Copy link
Copy Markdown
Author

Makes sense to me, seems like TPetra can be coerced into trying any kokkos version:
https://github.com/trilinos/Trilinos/blob/62ff542b83d63ad9a56b91ef7af177c485459dc2/packages/tpetra/CMakeLists.txt#L30-L53

@RaulPPelaez
Copy link
Copy Markdown
Author

Compiles fine but then fails with this:

  ERROR (trilinos,lib/libstokhos_muelu_mp_16_openmp.so.16.0.0): Needed DSO lib/libgomp.so.1 found in ['conda-forge/linux-64::_openmp_mutex==4.5=2_gnu']
  ERROR (trilinos,lib/libstokhos_muelu_mp_16_openmp.so.16.0.0): .. but ['conda-forge/linux-64::_openmp_mutex==4.5=2_gnu'] not in reqs/run, (i.e. it is overlinking) (likely) or a missing dependency (less likely)

@h-vetinari
Copy link
Copy Markdown
Member

Just add

  - libgomp      # [linux]
  - llvm-openmp  # [osx]

in host, those should have the right run-exports.

@RaulPPelaez
Copy link
Copy Markdown
Author

Trying to build using conda-forge's kokkos-kernels, seems to fail finding it:

CMake Error at cmake/tribits/core/package_arch/TribitsAddLibrary.cmake:472 (target_link_libraries):
  Target "zadelus" links to:

    KokkosKernels::all_libs

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.

Probably due to the kokkos-kernels feedstock being at version 4.1. Perhaps that should be updated before moving on here. But that feedstock also looks abandoned.

@rfiorella
Copy link
Copy Markdown
Contributor

Updated version in #58, if other features are still desired please feel free to start a new PR.

@rfiorella rfiorella closed this Aug 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants