Catch both ValueError and TypeError from pysynphot in _getSynphotBandpass

[josePhoenix]

Addresses #97.

Unfortunately, it doesn't look like there's an easy way to test just this code path. I tried something with py.test monkeypatching but rapidly found myself mucking with import machinery.

I think the proper solution is to additionally run CI in the affected configuration. With pysynphot installed and no PYSYN_CDBS environment variable set, a whole lot of tests were failing.

[mperrin]

Yes I agree that we should add some Travis cases with pysynphot. One with no data files would be good, plus perhaps one with a minimal data file set as well? I recall we talked about adding such a test case in the past. Any chance you could take a shot at adding that to the travis config?

[josePhoenix]

Yeah, I'll look into it. The question of what a minimal CDBS data set is may be tricky. I guess I can just watch the syscalls made for a single calculation to see what files it touches?

[mperrin]

Maybe @cslocum could comment here, or could direct us to someone in SSB who could advise. We shouldn't have to reverse-engineer from scratch what a minimal set of CDBS files is for continuous integration, not given that we're in the same building as the developers! :-)

[josePhoenix]

I couldn't resist trying to make a one-liner for it... Impressive how many files it touches. (Of course, we'll need more than just these files to test the stellar spectral types selection and whatnot.)

PYSYN_CDBS=/grp/hst/cdbs/ strace python -c "import pysynphot; pysynphot.ObsBandpass('wfc3,im,f120w')" 2>&1 | grep "/grp/hst/cdbs/" | grep "stat("
stat("/grp/hst/cdbs/extinction", {st_mode=S_IFDIR|S_ISGID|0755, st_size=1024, ...}) = 0
stat("/grp/hst/cdbs/extinction", {st_mode=S_IFDIR|S_ISGID|0755, st_size=1024, ...}) = 0
stat("/grp/hst/cdbs/extinction/milkyway_rv4_001.fits", {st_mode=S_IFREG|0644, st_size=8640, ...}) = 0
stat("/grp/hst/cdbs/extinction/xgal_starburst_001.fits", {st_mode=S_IFREG|0654, st_size=40320, ...}) = 0
stat("/grp/hst/cdbs/extinction/milkyway_dense_001.fits", {st_mode=S_IFREG|0654, st_size=8640, ...}) = 0
stat("/grp/hst/cdbs/extinction/lmc_diffuse_001.fits", {st_mode=S_IFREG|0654, st_size=8640, ...}) = 0
stat("/grp/hst/cdbs/extinction/lmc_30dorshell_001.fits", {st_mode=S_IFREG|0654, st_size=8640, ...}) = 0
stat("/grp/hst/cdbs/extinction/milkyway_diffuse_001.fits", {st_mode=S_IFREG|0654, st_size=8640, ...}) = 0
stat("/grp/hst/cdbs/extinction/milkyway_rv21_001.fits", {st_mode=S_IFREG|0644, st_size=8640, ...}) = 0
stat("/grp/hst/cdbs/extinction/smc_bar_001.fits", {st_mode=S_IFREG|0654, st_size=8640, ...}) = 0
stat("/grp/hst/cdbs/mtab/yah1742qm_tmg.fits", {st_mode=S_IFREG|0664, st_size=492480, ...}) = 0

[mperrin]

Huh wait what? Those files don't make any sense! Most of them are various models for interstellar extinction, which have no business whatsoever being anywhere near a calculation of a single WFC3 filter bandpass.

The last file appears to be more reasonable, a synphot graph table. FITS headers says last edited by Sean Lockwood so you could ask him to explain what it is.

[josePhoenix]

@mperrin actually, that's a bit of a red herring. It's not even a valid obsmode! If I change the string to '', it spits out the same list of files.

@sean-lockwood, maybe you have some ideas? I'm not well versed in CDBS

[mperrin]

It turns out we have already had "setup a Travis instance with pysynphot and a minimal CDBS file set" as issue #51 for over a year. :-)

[josePhoenix]

Yeah, I knew it was in the backlog somewhere... I think my horrendous strace hack could get us a minimal set of files, though.

[sean-lockwood]

The TMG ("The Master Graph") file is used to map the instrument obsmode string (consisting of comma-delimited "keywords") into the corresponding list of compnames. The TMC (component lookup table) file converts the compnames to actual file names. For completeness, some instruments have a TMT file that corresponds to the TMC file, but is used for thermal emission instead of throughput.

Depending on how the graph is defined, there are often default compnames that get added to the complist. There are also optional keywords that perform interpolation/extrapolation (e.g. "mjd#57423" tells Pysynphot to interpolate between the two neighboring MJD columns in the throughput file.)

There's a manual for Synphot that explains some of the Pysynphot files/concepts: http://www.stsci.edu/institute/software_hardware/stsdas/synphot/SynphotManual.pdf

I'll be back at the institute Monday, or Vicki Laidler is a good resource for Pysynphot matters. I have an IDL routine to examine the graph & component file (there are also utilities in the Synphot Pyraf package), and can help you construct an obsmode string for your testing purposes. (Or, maybe you want a synthetic set of TMG, TMC, and throughput files?). Let me know what you're looking for.

-Sean

[josePhoenix]

Thanks @sean-lockwod! I think we want a minimal set of files to run our test suite, and the determination of said subset may have to involve both a CDBS expert and myself or Marshall. I'll follow up on Monday.

[mperrin]

Well, since webbpsf contains its own reference tables for the normalized JWST instrument throughputs that it turns into ObsBandpasses, we don't actually need any instrument graph tables. (At one point years ago draft versions of these existed for the JWST instruments, but I don't know if Pandeia even uses those internally any more.)

It would be convenient to grab one or two stellar atmosphere model files (e.g. a file or two from the phoenix model grid) so we can make a test case using those. Though we could also cut that out and make test cases using black body spectra.

The real question is, what minimalist file set is needed so that pysynphot doesn't just immediately give up on import, a la TypeError: initializing GraphTable with GFile=None; possible bad/missing CDBS.

[mperrin]

Moving subsequent comments to #51