fastMRI icon indicating copy to clipboard operation
fastMRI copied to clipboard

conda-forge build of fastmri?

Open gschramm opened this issue 1 year ago • 6 comments

Please:

  • [x] Check for duplicate requests.
  • [x] Describe your goal, and if possible provide a code snippet with a motivating example.

Hi fastmri developers,

what about building fastmri also as conda-forge package? Since it is pure python and has only minimal dependencies, creating a conda-forge recipe should be easy - and obviously I am willing to help.

Georg

gschramm avatar Dec 12 '23 07:12 gschramm

Hello @gschramm, that sounds great! At this point I am mostly maintaining the repo and not adding features, but I could review your implementation if you'd like to work on it.

mmuckley avatar Dec 12 '23 14:12 mmuckley

Sounds great. Before creating the conda-forge "recipe" a few short questions:

  • Which pytorch version required?
  • Which of those tests should be run when building the package?

gschramm avatar Dec 12 '23 14:12 gschramm

Sounds great. Before creating the conda-forge "recipe" a few short questions:

* Which pytorch version required?

* Which of [those tests](https://github.com/facebookresearch/fastMRI/tree/main/tests) should be run when building the package?

Just found the requirements for install / testing here

gschramm avatar Dec 12 '23 14:12 gschramm

@mmuckley I did a generate a first conda-forge fastmri recipe using grayskull based on the pypi package. Unfortunately, it seems that the run requirement runstats is not (yet) on conda-forge. If this is an essential requirement that cannot be replaced, we have to get it on conda-forge first.

gschramm avatar Dec 12 '23 16:12 gschramm

@mmuckley conda-forge build is on its way :) Why do you have different reqs for install and testing in setup.cfg? Right now some tests in test_modules.py fail with newer versions of pytorch-lightning and torch-vision which seems dangerous to me (the install reqs. allows newer versions ...)

Are the tests in tests_modules.py essential or can they be skipped?

gschramm avatar Dec 17 '23 12:12 gschramm

Hello @gschramm, happy new year :).

I apologize for not replying faster - I was on vacation at the end of the year, and the first week of this year has been busy with performance reviews.

The different requirements are because we want a standard test bed to evaluate all aspects of the package, but we don't want to impose those same packages on users. Usually users will want to choose their own package versions.

Since we do distribute the PyTorch Lightning code we probably should get that working or move it into an examples folder. At this point I might prefer to keep it where it is, since it's been there awhile and there might be people building off of it. In that case, we'd have to update that code for the current version of PyTorch Lightning.

If you want, you can open your PR with those tests breaking and we can look at everything else, and maybe I could spend some time at some point to fix PyTorch Lightning (... again.... :( ).

mmuckley avatar Jan 09 '24 23:01 mmuckley