desihub / desisurvey

Code for desi survey planning and implementation
BSD 3-Clause "New" or "Revised" License
2 stars 7 forks source link

desisurvey.ephem is broken in current DESI releases #147

Open weaverba137 opened 2 years ago

weaverba137 commented 2 years ago

With desimodules/22.1b (astropy==5.0; desisurvey==0.18.0):

from desisurvey.ephem import get_ephem
ephem = get_ephem()
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
/tmp/ipykernel_23442/4078409922.py in <module>
     41 # Initialize ephemerides, to find Moon, etc.
     42 #
---> 43 ephem = get_ephem()
     44 #
     45 # Working directory.

/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desisurvey/0.18.0/lib/python3.9/site-packages/desisurvey/ephem.py in get_ephem(use_cache, write_cache)
     73         return _ephem
     74     # Next check for a FITS file on disk.
---> 75     config = desisurvey.config.Configuration()
     76     filename = config.get_path('ephem_{}_{}.fits'.format(start_iso, stop_iso))
     77     if use_cache and os.path.exists(filename):

/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desisurvey/0.18.0/lib/python3.9/site-packages/desisurvey/config.py in __new__(cls, file_name)
    154         if Configuration.__instance is None:
    155             Configuration.__instance = object.__new__(cls)
--> 156             Configuration.__instance._initialize(file_name)
    157         elif file_name is not None and file_name != Configuration.__instance.file_name:
    158             raise RuntimeError('Configuration already loaded from {0}'

/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desisurvey/0.18.0/lib/python3.9/site-packages/desisurvey/config.py in _initialize(self, file_name)
    187         self.file_name = file_name
    188 
--> 189         full_path = self._get_full_path(file_name)
    190 
    191         # Validate that all mapping keys are valid python identifiers

/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desisurvey/0.18.0/lib/python3.9/site-packages/desisurvey/config.py in _get_full_path(file_name)
    142         # Locate the config file in our pkg data/ directory if no path is given.
    143         if os.path.split(file_name)[0] == '':
--> 144             full_path = astropy.utils.data._find_pkg_data_path(
    145                 os.path.join('data', file_name))
    146         else:

AttributeError: module 'astropy.utils.data' has no attribute '_find_pkg_data_path'
weaverba137 commented 2 years ago

With desimodules/22.1b and module switch desisurvey/master:

from desisurvey.ephem import get_ephem
ephem = get_ephem()
INFO:iers.py:82:freeze_iers: Freezing IERS table used by astropy time, coordinates.
Traceback (most recent call last):
  File "/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desisurvey/master/py/desisurvey/config.py", line 242, in set_output_path
    self._output_path = output_path.format(**os.environ)
KeyError: 'DESISURVEY_OUTPUT'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desisurvey/master/py/desisurvey/ephem.py", line 76, in get_ephem
    filename = config.get_path('ephem_{}_{}.fits'.format(start_iso, stop_iso))
  File "/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desisurvey/master/py/desisurvey/config.py", line 276, in get_path
    self.set_output_path(self.output_path())
  File "/global/common/software/desi/cori/desiconda/20211217-2.0.0/code/desisurvey/master/py/desisurvey/config.py", line 244, in set_output_path
    raise ValueError(
ValueError: Environment variable not set for output_path: 'DESISURVEY_OUTPUT'
weaverba137 commented 2 years ago

So two different failure modes!

weaverba137 commented 2 years ago

Setting DESISURVEY_OUTPUT to some arbitrary value (I used $CSCRATCH) circumvents the problem with desisurvey/master, and ephem = get_ephem() then works.

However, there are still two serious problems:

sbailey commented 2 years ago

Why isn't DESISURVEY_OUTPUT set when the desisurvey module is loaded?

I'd prefer the code to be robust to $DESISURVEY_OUTPUT not being set, defaulting to something like using the current directory. If it really must be set for some functionality, I'd prefer it to at least not break imports, and generate a meaningful exception if the code that requires it is run.

Why hasn't a new tag been created that incorporates astropy 5 fixes?

Oversight, not intention

schlafly commented 2 years ago

In this case with the recent tag the failure is at get_ephem() stage, not at import stage.

I agree that DESISURVEY_OUTPUT could default to the current directory if we wanted that. This particular operation will be quite slow unless you're using a cached ephemerides file in DESISURVEY_OUTPUT, though (5 min?), and most existing code using this immediately writes out the ephemirides to a file which is then loaded in the future. So it's not so crazy for it to complain that DESISURVEY_OUTPUT isn't set.

weaverba137 commented 2 years ago

Is there a standard ephem file at NERSC, say used by survey operations?

weaverba137 commented 1 year ago

@sbailey, this may be related to your question today about *SURVEY* environment variables.