Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-800

Add third party package Spectractor to lsst_distrib

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      Add the third party package, Spectractor, to lsst_distrib.

      Spectractor is the core spectroscopic reduction part of the AuxTel pipeline, and the package atmospec, being added via RFC-799, provides an interface between DM code and Spectractor.

      Spectractor has several dependencies which are not yet in the stack, which would need to be added to our conda env. For Spectractor these are normally installed via pip, but all are available in conda, and I think are even all on the conda-forge channel now too.

      We currently run a fork, such that we have full control over versioning and updates etc, but I'd hope to make a PR to upstream once we make the requisite changes to our fork such that the packaging mechanisms remain in sync.

       

        Attachments

          Issue Links

            Activity

            Hide
            ktl Kian-Tat Lim added a comment - - edited

            Concerns for the dependencies include pysynphot, which does not appear to be in conda or conda-forge; the inclusion of test-only utilities like nose and coverage; and the pins on h5py, iminuit, and emcee, the first two of which are a major version behind what we currently have in rubin-env and the third of which is an RC of a version that's a minor version behind current.

            I have not yet looked into the size and stability of the other new dependencies.

            It may be that we need a separate CPP-only env? Oh. Except you're going to want to use this along side all other stack code in Summit notebook containers, aren't you. So I don't think that's possible.

            Show
            ktl Kian-Tat Lim added a comment - - edited Concerns for the dependencies include pysynphot , which does not appear to be in conda or conda-forge; the inclusion of test-only utilities like nose and coverage ; and the pins on h5py , iminuit , and emcee , the first two of which are a major version behind what we currently have in rubin-env and the third of which is an RC of a version that's a minor version behind current. I have not yet looked into the size and stability of the other new dependencies. It may be that we need a separate CPP-only env? Oh. Except you're going to want to use this along side all other stack code in Summit notebook containers, aren't you. So I don't think that's possible.
            Hide
            mfisherlevine Merlin Fisher-Levine added a comment -

            Ah, big apologies - I totally forgot to include the proposed branch which has fixed many of those issues - well, at least the pinning ones. Proposed branch is DM-29598, so the requirements for that are:

            numpy>1.15
            scipy
            matplotlib>3.1
            pandas
            llvmlite
            numba
            astropy>=3.2
            photutils>=0.7
            astroquery
            coloredlogs
            scikit-image
            h5py
            emcee
            tqdm
            mpi4py
            schwimmbad
            iminuit>=2
            coverage>=3.6
            configparser
            coveralls
            pysynphot
            deprecated
            pyyaml
            nose

            which hopefully looks a lot more reasonable.

            If the test-only parts are a real problem then I'm sure I can disable to built-in testing for our fork, though if it's not a serious issue being able not to diverge from upstream would be a big plus overall.

            I am not sure what to do about pysynphot though, that's potentially a real problem. I suspect it's already being pip-installed by users on the summit though for basic querying though, as it's a very useful utility for observers, so perhaps we should look into that quite independently of this anyway...

             

            Show
            mfisherlevine Merlin Fisher-Levine added a comment - Ah, big apologies - I totally forgot to include the proposed branch which has fixed many of those issues - well, at least the pinning ones. Proposed branch is DM-29598 , so the requirements for that are: numpy>1.15 scipy matplotlib>3.1 pandas llvmlite numba astropy>=3.2 photutils>=0.7 astroquery coloredlogs scikit-image h5py emcee tqdm mpi4py schwimmbad iminuit>=2 coverage>=3.6 configparser coveralls pysynphot deprecated pyyaml nose which hopefully looks a lot more reasonable. If the test-only parts are a real problem then I'm sure I can disable to built-in testing for our fork, though if it's not a serious issue being able not to diverge from upstream would be a big plus overall. I am not sure what to do about pysynphot though, that's potentially a real problem. I suspect it's already being pip-installed by users on the summit though for basic querying though, as it's a very useful utility for observers, so perhaps we should look into that quite independently of this anyway...  
            Hide
            ktl Kian-Tat Lim added a comment -

            Another small worry about pysynphot might be its use of external data. At least it's manually transferred and updated, but the tarballs are a few GB compressed and 8.2 GB expanded. Per-user copies are possible, but adding this to the shared stack code to maintain (and doing something similar in other instances) could be beneficial.

            Show
            ktl Kian-Tat Lim added a comment - Another small worry about pysynphot might be its use of external data. At least it's manually transferred and updated, but the tarballs are a few GB compressed and 8.2 GB expanded. Per-user copies are possible, but adding this to the shared stack code to maintain (and doing something similar in other instances) could be beneficial.
            Hide
            mfisherlevine Merlin Fisher-Levine added a comment -

            PR for adding pysynphot to conda-forge just filed here.

            Show
            mfisherlevine Merlin Fisher-Levine added a comment - PR for adding pysynphot to conda-forge just filed here .
            Hide
            tjenness Tim Jenness added a comment -

            Merlin Fisher-Levine please adopt this RFC and link to an "is triggering" work ticket.

            Show
            tjenness Tim Jenness added a comment - Merlin Fisher-Levine please adopt this RFC and link to an "is triggering" work ticket.
            Hide
            mfisherlevine Merlin Fisher-Levine added a comment -

            Sorry, was waiting for the weekly to be cut so as not to scary people with potential package changes just before the release was cut. Done and linked now.

            Show
            mfisherlevine Merlin Fisher-Levine added a comment - Sorry, was waiting for the weekly to be cut so as not to scary people with potential package changes just before the release was cut. Done and linked now.
            Hide
            tjenness Tim Jenness added a comment -

            Adoption just means you have decided that the RFC should be implemented. It doesn't mean you are going to implement it immediately.

            Show
            tjenness Tim Jenness added a comment - Adoption just means you have decided that the RFC should be implemented. It doesn't mean you are going to implement it immediately.

              People

              Assignee:
              mfisherlevine Merlin Fisher-Levine
              Reporter:
              mfisherlevine Merlin Fisher-Levine
              Watchers:
              Kian-Tat Lim, Merlin Fisher-Levine, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  Jenkins

                  No builds found.