conda-forge / petsc-feedstock

A conda-smithy repository for petsc.
BSD 3-Clause "New" or "Revised" License
8 stars 23 forks source link

support for FFTW #113

Closed MarDiehl closed 3 years ago

MarDiehl commented 3 years ago

@conda-forge-admin, please rerender

Checklist

conda-forge-linter commented 3 years ago

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.

dalcinl commented 3 years ago

@MarDiehl LGTM. Please review a few build logs to double check. Ping me back once ready and I'll merge.

dalcinl commented 3 years ago

@MarDiehl Perhaps you need to specify a variant string to use MPI-enabled FFT builds?

dalcinl commented 3 years ago

Or perhaps via conda_build_config.yml, as done with hdf5 for the MPI variant.

MarDiehl commented 3 years ago

Or perhaps via conda_build_config.yml, as done with hdf5 for the MPI variant.

I think this is the correct way. I'll give it a try

dalcinl commented 3 years ago

@isuruf Could you please tell us what's the proper way to make this recipe build and depend on the MPI-enabled FFTW package?

MarDiehl commented 3 years ago

Looks ok now, the only failed test is the result of a timeout. Anything do do from my side?

dalcinl commented 3 years ago

@MarDiehl Let's start fresh, revert all of your changes to the meta.yaml and conda_build_config.yaml, and specify the fftw package the following way

requirements:
  host:
    - fftw * *mpi_{{ mpi }}*
  run:
    - fftw * *mpi_{{ mpi }}*
MarDiehl commented 3 years ago

@MarDiehl Let's start fresh, revert all of your changes to the meta.yaml and conda_build_config.yaml, and specify the fftw package the following way

requirements:
  host:
    - fftw * *mpi_{{ mpi }}*
  run:
    - fftw * *mpi_{{ mpi }}*

unfortunately, it does not work. Do you have any objection against the last working version? I would then put it into a clean branch or force push (not sure if that works)

dalcinl commented 3 years ago

unfortunately, it does not work. Do you have any objection against the last working version?

Could you clarify? You latest push looks exactly like my last suggestion, plus the # [ mpi != 'nompi'] comment I do not understand why we need it.

MarDiehl commented 3 years ago

unfortunately, it does not work. Do you have any objection against the last working version?

Could you clarify? You latest push looks exactly like my last suggestion, plus the # [ mpi != 'nompi'] comment I do not understand why we need it.

I copied your suggestion and it failed. But currently we have fftw * mpi_{{ mpi }}_* # [mpi != 'nompi'] while your suggestion was fftw * *mpi_{{ mpi }}* which differs not only with respect to the comment but also with respect to double * and trailing _. I'm new to Conda and only have a rough understanding what the code does. Should I try the current version and just strip the comment?

dalcinl commented 3 years ago

I copied your suggestion and it failed. But currently we have fftw * mpi_{{ mpi }}_* # [mpi != 'nompi'] while your suggestion was fftw * *mpi_{{ mpi }}* which differs not only with respect to the comment but also with respect to double * and trailing _.

Oh, sorry, yes! My bad.

I'm new to Conda and only have a rough understanding what the code does.

You are not alone! I also barely know the low level details. It is really frustrating to be a recipe maintainer. I'm truly incompetent. I asked more than once for help (to conda-forge outsiders), but evidently those I asked help for are way smarter than I am.

Should I try the current version and just strip the comment?

Can we please try without the comment? It should not be required, I'm confident it should work. I do not want confusing leftovers in the recipe, because maintenance is already hard for me, not because of time, but because of understanding.

MarDiehl commented 3 years ago

works without the dubious # [mpi != 'nompi'].

dalcinl commented 3 years ago

@MarDiehl Many thanks for your help and patience!

MarDiehl commented 3 years ago

@MarDiehl Many thanks for your help and patience!

Thank you, for the feedstock and for PETSc in general!