desihub / desispec

DESI spectral pipeline
BSD 3-Clause "New" or "Revised" License
35 stars 24 forks source link

Fix #2138. #2154

Closed moustakas closed 7 months ago

moustakas commented 7 months ago

Tentative fix for #2138 but we'll need to confirm if this fixes this part of the issue you identified, @sbailey:

@moustakas we're getting test_photo failures at NERSC, e.g. see https://data.desi.lbl.gov/desi/spectro/redux/dailytest/log/perlmutter/desispec.log . 
When run as part of the full test suite, I'm getting one failure like

...
        if legacysurveydir is None:
            legacysurveydir = os.path.join(desi_root, 'external', 'legacysurvey', 'dr9')

        if not os.path.isdir(legacysurveydir):
            errmsg = f'Legacy Surveys directory {legacysurveydir} not found.'
            log.critical(errmsg)
>           raise IOError(errmsg)
E           OSError: Legacy Surveys directory /tmp/tmpgio7la0q/external/legacysurvey/dr9 not found.
and two failures like

        fiberfile = os.path.join(fiberassign_dir, stileid[:3], f'fiberassign-{stileid}.fits.gz')
        if not os.path.isfile(fiberfile):
            fiberfile = fiberfile.replace('.gz', '')
            if not os.path.isfile(fiberfile):
                errmsg = f'Fiber assignment file {fiberfile} not found!'
                log.critical(errmsg)
>               raise IOError(errmsg)
E               OSError: Fiber assignment file /tmp/tmpgio7la0q/target/fiberassign/tiles/trunk/000/fiberassign-000019.fits not found!
It looks like some other test might be resetting $DESI_ROOT or $DESI_ROOT_READONLY out from under you. test_io does some $DESI_ROOT manipulation, but it also attempts to set it back when done.

The functions in desispec.io.photo as well as the test_photo suite relies entirely on desispec.io.meta.get_desi_root_readonly doing the right thing, so if another unit test is changing the top-level DESI path, then I'm going to need help tracking down who/what/where.

moustakas commented 7 months ago

With the latest commit at NERSC I get

% pytest py/desispec/test/test_photo.py
======================================================================================= test session starts =======================================================================================
platform linux -- Python 3.10.8, pytest-7.2.0, pluggy-1.0.0
rootdir: /global/u2/i/ioannis/code/desihub/desispec
plugins: astropy-header-0.1.2, filter-subpackage-0.1.1, openfiles-0.5.0, doctestplus-0.12.1, arraydiff-0.3, anyio-3.6.2, astropy-0.10.0, asdf-2.14.3, cov-4.0.0, remotedata-0.4.0, hypothesis-6.62.0, mock-3.10.0
collected 4 items

py/desispec/test/test_photo.py ....                                                                                                                                                         [100%]

======================================================================================= 4 passed in 25.24s ========================================================================================

The only new / different assumption I had to make was

#surveyops_dir = os.environ['DESI_SURVEYOPS']
surveyops_dir = desi_root+'/survey/ops/surveyops/trunk' # assumes a standard installation / environment

Hopefully this is alright @sbailey.

sbailey commented 7 months ago

Thanks. This PR fixes the second problem listed in #2138, i.e. the failure of test_photo.py when run by itself. Good. We still have the problem that some other test is changing DESI_ROOT and not resetting it, thus breaking test_photo.py when all of the tests are run together with a single pytest invocation. So I'll merge this PR now, but leave #2138 open until we can debug the other cross-test problem.