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

Adding cublasMp recipes #26580

Merged
merged 15 commits into from
Sep 10, 2024
Merged

Adding cublasMp recipes #26580

merged 15 commits into from
Sep 10, 2024

Conversation

adibbley
Copy link
Contributor

@adibbley adibbley commented Jun 6, 2024

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

This PR adds recipes for the NVIDIA Math Library cublasMp, which contains 2 library components: libcublasmp and libcal.

@conda-forge-webservices
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 (recipes/cublasmp, recipes/libcal, recipes/libcublasmp) and found some lint.

Here's what I've got...

For recipes/cublasmp:

  • 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.

For recipes/libcal:

  • The following maintainers have not yet confirmed that they are willing to be listed here: conda-forge/cuda. Please ask them to comment on this PR if they are.

For recipes/libcublasmp:

  • Selectors are suggested to take a <two spaces>#<one space>[<expression>] form. See lines [42, 47]
  • The following maintainers have not yet confirmed that they are willing to be listed here: conda-forge/cuda. Please ask them to comment on this PR if they are.

@conda-forge-webservices
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 (recipes/cublasmp, recipes/libcal, recipes/libcublasmp) and found some lint.

Here's what I've got...

For recipes/cublasmp:

  • 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.

For recipes/libcal:

  • The following maintainers have not yet confirmed that they are willing to be listed here: conda-forge/cuda. Please ask them to comment on this PR if they are.

For recipes/libcublasmp:

  • The following maintainers have not yet confirmed that they are willing to be listed here: conda-forge/cuda. Please ask them to comment on this PR if they are.

@adibbley
Copy link
Contributor Author

adibbley commented Jun 6, 2024

cc @conda-forge/cuda

@conda-forge-webservices
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 (recipes/cublasmp, recipes/libcal, recipes/libcublasmp) and found some lint.

Here's what I've got...

For recipes/cublasmp:

  • The following maintainers have not yet confirmed that they are willing to be listed here: conda-forge/cuda. Please ask them to comment on this PR if they are.

For recipes/libcal:

  • The following maintainers have not yet confirmed that they are willing to be listed here: conda-forge/cuda. Please ask them to comment on this PR if they are.

For recipes/libcublasmp:

  • The following maintainers have not yet confirmed that they are willing to be listed here: conda-forge/cuda. Please ask them to comment on this PR if they are.

@adibbley
Copy link
Contributor Author

adibbley commented Jun 6, 2024

The failures in osx and win are coming from the top level metapackage which is currently set to noarch: generic. This product doesn't support those platforms so I think this is expected.

@adibbley
Copy link
Contributor Author

adibbley commented Jun 6, 2024

The cuda 11.8 build seems to be failing with:

The following packages are incompatible
├─ __cuda is requested and can be installed;
└─ cuda-version 12.*  is not installable because it requires
   └─ __cuda >=12 , which conflicts with any installable versions previously reported.

I was getting the expected cuda packages for each version locally, so not sure if I need to tighten a constraint somewhere.

@leofang
Copy link
Member

leofang commented Jun 6, 2024

My impression is that in staged recipes only CUDA 11 is supported, so CUDA 12 can wait until we have feedstocks.

@kvoronin
Copy link

kvoronin commented Jun 6, 2024

Copy link
Member

Choose a reason for hiding this comment

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

Q: I wonder if it'd make things easier to make the cublasmp meta package an output of libcublasmp-feedstock? (No strong opinion, just curious if this has been considered.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keeping them separate helps with how we setup releases internally. The recipes would be updated with a similar process to CUDA, just on a smaller scale.

Copy link
Member

Choose a reason for hiding this comment

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

Do you not want run_exports for this package? Otherwise, what is the purpose of this package? No downstream packages should depend on this pacakge directly because they would get all of the devlopement files for libcal and libcublasmp but none of the run_exports.

Copy link
Member

Choose a reason for hiding this comment

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

Same question. What's the purpose of this package? Also, I don't see a compelling reason to keep this in a separate feedstock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This package is for convenient installation of all components, similar to the cuda metapackages.

Copy link
Member

Choose a reason for hiding this comment

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

Please add that this is not to be used in a conda-build context in the summary.
Also, please move this to libcublasmp recipe if there's not reason to keep this in a separate feedstock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a note now.
Having separate feedstocks helps us with maintenance by keeping the design consistent across products.

Copy link
Member

Choose a reason for hiding this comment

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

That doesn't make sense to me at all.

Copy link
Member

Choose a reason for hiding this comment

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

Can you explain in more detail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All CUDA metapackages are in separate feedstocks, I'm not seeing a reason to change that for cublasmp. Versions of these packages are not technically coupled, they are given a release label as seen in https://developer.download.nvidia.com/compute/cublasmp/redist/redistrib_0.2.1.json

    "release_label": "0.2.1",
    "release_product": "cublasmp",

recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
recipes/libcublasmp/meta.yaml Outdated Show resolved Hide resolved
recipes/libcublasmp/meta.yaml Outdated Show resolved Hide resolved
recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
recipes/cublasmp/meta.yaml Show resolved Hide resolved
recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
@leofang
Copy link
Member

leofang commented Jun 12, 2024

I am a bit confused by the log:

2024-06-12T20:40:07.7416928Z ERROR: Glob /home/conda/staged-recipes/build_artifacts/libcal_1718224708287/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_plac/share/ucc.conf did not match in root_dir /home/conda/staged-recipes/build_artifacts/libcal_1718224708287/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_plac

and

2024-06-12T20:40:30.5341271Z + test -f /home/conda/staged-recipes/build_artifacts/libcal_1718224708287/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold/share/libcal/ucc.conf
2024-06-12T20:40:32.0895642Z WARNING: Tests failed for libcal-dev-0.4.3.36-h4b97088_0.conda - moving package to /home/conda/staged-recipes/build_artifacts/broken

It seems as if the copy logic in top-level build:script: is not done correctly...

recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
@leofang
Copy link
Member

leofang commented Jun 14, 2024

It seems to be working fine, except for the osx/win CIs which are known to fail in stage-recipes if there's a generic/noarch recipe. @conda-forge/staged-recipes this is ready.

Copy link
Member

Choose a reason for hiding this comment

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

Do you not want run_exports for this package? Otherwise, what is the purpose of this package? No downstream packages should depend on this pacakge directly because they would get all of the devlopement files for libcal and libcublasmp but none of the run_exports.

recipes/libcublasmp/meta.yaml Outdated Show resolved Hide resolved
recipes/libcublasmp/meta.yaml Outdated Show resolved Hide resolved
recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
@leofang
Copy link
Member

leofang commented Jul 16, 2024

@kvoronin @mrogowski could you address the questions? Thanks.

recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
recipes/libcal/meta.yaml Outdated Show resolved Hide resolved
@mrogowski
Copy link
Contributor

mrogowski commented Aug 1, 2024

@kvoronin @mrogowski could you address the questions? Thanks.

I answered some of the questions. I hope @adibbley can answer the rest. Thanks!

adibbley and others added 2 commits August 6, 2024 13:08
Co-authored-by: Marcin Rogowski <[email protected]>
Co-authored-by: Daniel Ching <[email protected]>
@conda-forge-webservices
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 (recipes/cublasmp/meta.yaml, recipes/libcal/meta.yaml, recipes/libcublasmp/meta.yaml) and found some lint.

Here's what I've got...

For recipes/cublasmp/meta.yaml:

  • The following maintainers have not yet confirmed that they are willing to be listed here: conda-forge/cuda. Please ask them to comment on this PR if they are.

For recipes/libcal/meta.yaml:

  • The following maintainers have not yet confirmed that they are willing to be listed here: conda-forge/cuda. Please ask them to comment on this PR if they are.

For recipes/libcublasmp/meta.yaml:

  • The following maintainers have not yet confirmed that they are willing to be listed here: conda-forge/cuda. Please ask them to comment on this PR if they are.

@jakirkham
Copy link
Member

@conda-forge/staged-recipes can someone please take another look? 🙂

Copy link

To help direct your pull request to the best reviewers, please mention a topic-specifc team if your recipe matches any of the following: conda-forge/help-c-cpp, conda-forge/help-cdts, conda-forge/help-go, conda-forge/help-java, conda-forge/help-julia, conda-forge/help-nodejs, conda-forge/help-perl, conda-forge/help-python, conda-forge/help-python-c, conda-forge/help-r, conda-forge/help-ruby,or conda-forge/help-rust. Thanks!

@manopapad
Copy link

@conda-forge/help-c-cpp let's see if this works

@sisodia1701
Copy link

@conda-forge-admin, please ping team

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-webservice.

I was asked to ping @conda-forge/staged-recipes and so here I am doing that.

Copy link

To help direct your pull request to the best reviewers, please mention a topic-specifc team if your recipe matches any of the following: conda-forge/help-c-cpp, conda-forge/help-cdts, conda-forge/help-go, conda-forge/help-java, conda-forge/help-julia, conda-forge/help-nodejs, conda-forge/help-perl, conda-forge/help-python, conda-forge/help-python-c, conda-forge/help-r, conda-forge/help-ruby,or conda-forge/help-rust. Thanks!

@carterbox carterbox self-assigned this Sep 10, 2024
@carterbox carterbox merged commit 8d2e5e3 into conda-forge:main Sep 10, 2024
2 of 7 checks passed
@beckermr
Copy link
Member

This recipe is breaking staged recipes. It seems that the cuda compiler version variable in jinja2 needs a default definition.

@jakirkham
Copy link
Member

This was due to a since fixed conda-build bug

Have added more info in comment: #27547 (comment)

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

Successfully merging this pull request may close these issues.