synphot requirement

[Russell-Ryan]

poppy does not install the correct version of synphot (currently v1.1). It seems the setup.cfg file needs to have install_requires synphot>=1.1.0 ? the work around was to pip install synphot==1.1

[mperrin]

Thanks @Russell-Ryan. @shanosborne could you please take care of this? Thanks!

[shanosborne]

I can definitely add a version requirement to the setup.cfg file. However, I'm not sure we want to add it to install_requires.

We originally set up the code so that it wouldn't install synphot, because it's not technically needed to run the package - it's a bonus to get more features. That's why it lives under [options.extras_require] in the setup.cfg file.

@mperrin Do you want to keep the synphot installation optional? In that case I'd leave it in extras_require and I can just add the version requirement. Or I can move it to install_requires if we want it to install for everyone by default.

[Russell-Ryan]

@shanosborne I'm not sure, but I know that when we just a vanilla pip install . it did not set up synphot. Then we ran some code given to us by Charles Lajoie that crashed because synphot wasn't set up. In fact, we trace the problem to something in instrument.py, and the only way we could resolve the issue was to pip install synphot==1.1.

[mperrin]

Good questions Shannon. I haven't thought this through in any detail yet and to be honest wasn't intending any significant change in functionality, just a bump in version number.

It may be that the right fix is making synphot a required dependency of webbpsf, rather than poppy. For poppy it should probably stay optional? Not sure.

Having that be optional may be less needed now than used to be the case, before the split of stsynphot from synphot. Back in the old days of pysynphot, installing pysynphot required manually downloading several zip files of reference data, which didn't work at all well with automated installs via pip. Did that get fixed such that now plain synphot can be installed easily via pip or conda? Sounds like that from what Russell writes. If so maybe it's easiest and simplest to just make it a required dependency now.

[shanosborne]

According to the synphot docs

To use the pre-defined standard star, extinction laws, and bandpasses, it is recommended for non-internal STScI users to download the necessary data files to a local directory so you can avoid connecting directly to STScI HTTP service, which is slower and might not be available all the time.

So it seems like downloading any data files are optional, even if you're not at ST. But we do call the johnson filter data files in _get_filter_list in instrument.py

[mperrin]

@shanosborne we should also come back and resolve this as well, in prep for 1.0

[shanosborne]

@mperrin Right, so I found that you do still need data files for synphot, though you can access them through the ST HTTP service rather than being forced to download them (though it is slower). So I think the standing question is - do we want to require this, and if so for which package - webbpsf or poppy (or both?)

[mperrin]

I'd lean towards making synphot a required dependency of webbpsf, but leaving it as an optional dependency for poppy. poppy is much more general and has plenty of use cases that don't depend on synphot. Further, the poppy.Instrument class is already written with code to handle synphot as an optional dependency (all the _HAS_SYNPHOT parts).

[mperrin]

@Russell-Ryan can you provide more details on what error messages you encountered regarding synphot? I'm a little surprised, since I see that my local machine has synphot 1.0.1 and yet all the tests pass for poppy and webbpsf. So if there is any functionality which actually depends on synphot 1.1 or greater, it's not tested as part of our test suite. Which maybe is an omission that we should fix, but we'd need more information on what the issue is to do so. Thanks!

[Russell-Ryan]

@mperrin i don't recall the precise error, but i didn't have synphot installed at all.

[mperrin]

@Russell-Ryan posted in email:

So I ran Charles notebook with the version of the code and reference data pointed to me by Justin. Specifically, it was

webbpsf version: 0.9.2.dev286+gc75a455 poppy version: 0.9.2

And I got these errors on the 9th cell, which is in the Measure the FWHM block:

---------------------------------------------------------------------------

NameError                                 Traceback (most recent call last)
/var/folders/gr/8brz2dg52j7by0cg8ypprrn80004mg/T/ipykernel_37662/3253268425.py in <module>
     10         wfi.detector = idet
     11 
---> 12         wfi_psf = wfi.calc_psf(nlambda = 10, oversample = 4)
     13         fwhm[ndet, nfilt] = webbpsf.measure_fwhm(wfi_psf, ext = 0)
     14 

~/miniconda3/envs/webbpsf/lib/python3.8/site-packages/poppy/instrument.py in calc_psf(self, outfile, source, nlambda, monochromatic, fov_arcsec, fov_pixels, oversample, detector_oversample, fft_oversample, overwrite, display, save_intermediates, return_intermediates, normalize)
    247 
    248         # ----- compute weights for each wavelength based on source spectrum
--> 249         wavelens, weights = self._get_weights(source=source, nlambda=local_options['nlambda'],
    250                                               monochromatic=local_options['monochromatic'])
    251 

~/miniconda3/envs/webbpsf/lib/python3.8/site-packages/poppy/instrument.py in _get_weights(self, source, nlambda, monochromatic, verbose)
    886 
    887             poppy_core._log.info("Computing wavelength weights using synthetic photometry for %s..." % self.filter)
--> 888             band = self._get_synphot_bandpass(self.filter)
    889             # choose reasonable min and max wavelengths
    890             w_above10 = np.where(band.throughput > 0.10 * band.throughput.max())

~/Roman/WebbPSF/webbpsf_0.9/webbpsf/webbpsf_core.py in _get_synphot_bandpass(self, filtername)
    630         filterdata = filterfits[1].data
    631         try:
--> 632             band = synphot.SpectralElement(synphot.models.Empirical1D, points=filterdata.WAVELENGTH,
    633                                                lookup_table=filterdata.THROUGHPUT, keep_neg=False)
    634 

NameError: name 'synphot' is not defined

I don’t recall precisely the error we got with synphot in poppy last time, but it was something like this. I can probably resolve/work through these errors, but I wanted advice before doing so. Any thoughts?

[mperrin]

@Russell-Ryan First, as I suggested some weeks ago the fix for this is straightforward and obvious enough: making synphot a required rather than optional dependency for webbpsf.

That said I'm a little surprised that this error can arise given the code in poppy.... In particular the stack trace above shows the call to self._get_synphot_bandpass in poppy/instrument.py line 888. However just above that on line 865 there's a check for having synphot installed:

        elif _HAS_SYNPHOT and (isinstance(source, synphot.SourceSpectrum) or source is None):

So it should never get to that call if synphot isn't present. Hmm.

Oh, aha. I've got it. This is a transient inconsistency because of the pysynphot/synphot migration. The version of poppy you have, v0.9.2 is checking for pysynphot:

        elif _HAS_PYSYNPHOT and (isinstance(source, pysynphot.spectrum.SourceSpectrum) or source is None):

My guess is your system has pysynphot installed but not synphot. So the mixed case of an older poppy release (still looking for pysynphot) plus dev webbpsf (now looking for synphot) leads to this confusion.

[mperrin]

In any case, now that the mystery is cleared up, what shall we do about it, if anything?

On the one hand I assert that you could install dev poppy (instead of poppy 0.9.2) and that notebook should run without error, because both dev poppy and dev webbpsf are now consistently both looking for synphot instead of pysynphot.

On the other hand, I still think we should just make synphot a required dependency of webbpsf because it adds useful functionality, and the installation hassle of plain synphot is so much less than pysynphot.

@shanosborne what do you think?

[Russell-Ryan]

I have a fresh environment that I built this morning with justin. I didn't install any form of synphot or pysynphot, and only did pip install on the webbpsf codebase. Also, I didn't install poppy by hand, presuming the webbpsf install would catch it. I have since tried to do things like pip install synphot, pip install synphot==1.1, and pip install pysynphot. I've restarted the notebooks, and nothing resolves this issue.

To be clear, I had versions

0.9.2.dev286+gc75a455.d20211203 0.9.2

of webbpsf and poppy, respectively.

What can I do next to help?

[mperrin]

Hi Russell, my assertion is that if you also install the dev version of poppy, this error will go away. It results from the inconsistent versions of poppy and webbpsf you have currently.

[Russell-Ryan]

Ok. Im not sure how to do that?

R

Sent from my iPhone

On Dec 3, 2021, at 3:36 PM, Marshall Perrin @.***> wrote:



Hi Russell, my assertion is that if you also install the dev version of poppy, this error will go away. It results from the inconsistent versions of poppy and webbpsf you have currently.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/spacetelescope/poppy/issues/464*issuecomment-985811369__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!laGIEwX7roDsUrtvkvIYa0MIt50gJVarM63xpNNjB33PG5dJam40Ut78kXtyoA$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ACGLBMEKXQEAARLNRHESXJDUPES4HANCNFSM5HHEDCAA__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!laGIEwX7roDsUrtvkvIYa0MIt50gJVarM63xpNNjB33PG5dJam40Ut5grtuSHA$. Triage notifications on the go with GitHub Mobile for iOShttps://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!laGIEwX7roDsUrtvkvIYa0MIt50gJVarM63xpNNjB33PG5dJam40Ut572mwPgA$ or Androidhttps://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign*3Dnotification-email*26utm_medium*3Demail*26utm_source*3Dgithub__;JSUlJSU!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!laGIEwX7roDsUrtvkvIYa0MIt50gJVarM63xpNNjB33PG5dJam40Ut7E1Rve9Q$.

[ojustino]

Hi @Russell-Ryan, did you see number 3 on poppy's installation guide? You want to make sure you're in the same conda environment you created to test my Cycle 9 pull request -- I believe you named it webbpsf?

[Russell-Ryan]

I didn’t install poppy by hand, I only did pip install on webbpsf. Does that installer not auto-install the relevant dependency?

On Dec 5, 2021, at 3:18 PM, ojustino @.@.>> wrote:

Hi @Russell-Ryanhttps://urldefense.com/v3/__https://github.com/Russell-Ryan__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!nirBTILpnQT9EzSnnoeoqXdnCWrQUksT26IsY1TilFTcGnmQnCHCKxLBpJDq5Q$, did you see number 3 on poppy's installation guidehttps://urldefense.com/v3/__https://poppy-optics.readthedocs.io/en/latest/installation.html*installation__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!nirBTILpnQT9EzSnnoeoqXdnCWrQUksT26IsY1TilFTcGnmQnCHCKxItT1igew$? You want to make sure you're in the same conda environment you created to test my Cycle 9 pull request -- I believe you named it webbpsf?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/spacetelescope/poppy/issues/464*issuecomment-986293320__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!nirBTILpnQT9EzSnnoeoqXdnCWrQUksT26IsY1TilFTcGnmQnCHCKxK4he72AA$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ACGLBMGN4PASTAA35A6JSELUPPCIPANCNFSM5HHEDCAA__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!nirBTILpnQT9EzSnnoeoqXdnCWrQUksT26IsY1TilFTcGnmQnCHCKxJk-ZOxVw$. Triage notifications on the go with GitHub Mobile for iOShttps://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!nirBTILpnQT9EzSnnoeoqXdnCWrQUksT26IsY1TilFTcGnmQnCHCKxIusjyIyw$ or Androidhttps://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign*3Dnotification-email*26utm_medium*3Demail*26utm_source*3Dgithub__;JSUlJSU!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!nirBTILpnQT9EzSnnoeoqXdnCWrQUksT26IsY1TilFTcGnmQnCHCKxIi-N45Mg$.

[ojustino]

You do have a version of poppy, but Marshall has asked you to install the development version, which happens via the method linked in my previous post.

[Russell-Ryan]

Ok. I can do that. But two questions.

1. is that the normal installation procedure? I mean we don’t expect users to do this, do we?

2. you’re sure that points to the dev version? It looks like it’s just to the latest, and the latest is dev (for example, dev isn’t it’s own branch or fork?)

R

Sent from my iPhone

On Dec 5, 2021, at 6:32 PM, ojustino @.***> wrote:



You do have a version of poppy, but Marshall has asked you to install the development version, which happens via the method linked in my previous post.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/spacetelescope/poppy/issues/464*issuecomment-986323355__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!n-mWCzCNOtRSuBZ7wqiYBkKi1LXk10vixOoBrjMzdI_KIm1cYN_1SSk79gdMfQ$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ACGLBMFCNHPG4RYBTW7UKNDUPPZADANCNFSM5HHEDCAA__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!n-mWCzCNOtRSuBZ7wqiYBkKi1LXk10vixOoBrjMzdI_KIm1cYN_1SSnO6CFxPA$. Triage notifications on the go with GitHub Mobile for iOShttps://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!n-mWCzCNOtRSuBZ7wqiYBkKi1LXk10vixOoBrjMzdI_KIm1cYN_1SSm4Bhyckw$ or Androidhttps://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign*3Dnotification-email*26utm_medium*3Demail*26utm_source*3Dgithub__;JSUlJSU!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!n-mWCzCNOtRSuBZ7wqiYBkKi1LXk10vixOoBrjMzdI_KIm1cYN_1SSkGVZCiFA$.

[mperrin]

Hi, I can advise more with this tomorrow. Russell no we don’t expect users to install the dev version; rather the way this works is poppy and WebbPSF have synchronized releases. So we will do a release of poppy, then the release of WebbPSF will depend on that version of poppy. They get developed side by side more or less, and each WebbPSF release assumes it goes with the latest matching poppy release. The reason you’re having trouble is that your setup doesn’t have that match: you’re using a dev webbpsf with an older release poppy.

[Russell-Ryan]

Ok... I follow justin's instructions on getting the dev of poppy. I now have these versions:

webbpsf: 0.9.2.dev286+gc75a455.d20211203 poppy: 1.1.dev228+gca97b1d

But get a different error when using wfi.calc_psf() from Charles' notebook.

---

ValueError Traceback (most recent call last) /var/folders/gr/8brz2dg52j7by0cg8ypprrn80004mg/T/ipykernel_95290/3253268425.py in 10 wfi.detector = idet 11 ---> 12 wfi_psf = wfi.calc_psf(nlambda = 10, oversample = 4) 13 fwhm[ndet, nfilt] = webbpsf.measure_fwhm(wfi_psf, ext = 0) 14

~/Roman/WebbPSF/poppy_dev/poppy/poppy/instrument.py in calc_psf(self, outfile, source, nlambda, monochromatic, fov_arcsec, fov_pixels, oversample, detector_oversample, fft_oversample, overwrite, display, save_intermediates, return_intermediates, normalize) 245 246 # ----- compute weights for each wavelength based on source spectrum --> 247 wavelens, weights = self._get_weights(source=source, nlambda=local_options['nlambda'], 248 monochromatic=local_options['monochromatic']) 249

~/Roman/WebbPSF/poppy_dev/poppy/poppy/instrument.py in _get_weights(self, source, nlambda, monochromatic, verbose) 919 f"with width {deltawave.to(units.micron):.2f}") 920 box = SpectralElement(Box1D, amplitude=1, x_0=wave, width=deltawave) * band --> 921 if box.tpeak() == 0: 922 # watch out for pathological cases with no overlap (happens with MIRI FND at high nlambda) 923 result = 0.0

~/miniconda3/envs/webbpsf/lib/python3.8/site-packages/synphot/spectrum.py in tpeak(self, wavelengths) 1768 1769 """ -> 1770 x = self._validate_wavelengths(wavelengths) 1771 return self(x).max() 1772

~/miniconda3/envs/webbpsf/lib/python3.8/site-packages/synphot/spectrum.py in _validate_wavelengths(self, wave) 340 """Validate wavelengths for sampling.""" 341 if wave is None: --> 342 if self.waveset is None: 343 raise exceptions.SynphotError( 344 'self.waveset is undefined; '

~/miniconda3/envs/webbpsf/lib/python3.8/site-packages/synphot/spectrum.py in waveset(self) 318 def waveset(self): 319 """Optimal wavelengths for sampling the spectrum or bandpass.""" --> 320 w = get_waveset(self.model) 321 if w is not None: 322 utils.validate_wavelengths(w)

~/miniconda3/envs/webbpsf/lib/python3.8/site-packages/synphot/models.py in get_waveset(model) 897 waveset = _model_tree_evaluate_sampleset_compat(model) 898 else: --> 899 waveset = _model_tree_evaluate_sampleset(model) 900 901 return waveset

~/miniconda3/envs/webbpsf/lib/python3.8/site-packages/synphot/models.py in _model_tree_evaluate_sampleset(root) 810 # Combine sampleset from both models. 811 else: --> 812 w1 = _model_tree_evaluate_sampleset(model1) 813 w2 = _model_tree_evaluate_sampleset(model2) 814 w = merge_wavelengths(w1, w2)

~/miniconda3/envs/webbpsf/lib/python3.8/site-packages/synphot/models.py in _model_tree_evaluate_sampleset(root) 784 # Not a CompoundModel, grab sampleset and be done. 785 if not hasattr(root, 'op'): --> 786 return _get_sampleset(root) 787 788 model1 = root.left

~/miniconda3/envs/webbpsf/lib/python3.8/site-packages/synphot/models.py in _get_sampleset(model) 777 w = None 778 if isinstance(model, Model) and hasattr(model, 'sampleset'): --> 779 w = model.sampleset() 780 return w 781

~/miniconda3/envs/webbpsf/lib/python3.8/site-packages/synphot/models.py in sampleset(self, step, minimal) 216 217 """ --> 218 w1, w2 = self.bounding_box 219 220 if self._n_models == 1:

ValueError: not enough values to unpack (expected 2, got 1)

[mperrin]

OK, that error is due to a bug in synphot, in which prior versions are not compatibly with astropy 5.0. The required fix is to have the most recent synphot, version 1.1.1, if you have astropy 5.0 installed. This is an upstream bug and is not something we can fix within WebbPSF or poppy. Try updating your synphot.

This is a very recent problem - astropy 5.0 was only released a couple weeks ago, and the synphot 1.1.1 fix a few days later. In the case of WebbPSF we can't strictly require astropy 5.0 or synphot 1.1.1 because those are literally just weeks old, and we to choose support not just that bleeding edge; we have to also support older versions of code from ~1 year ago, in particular astropy 4.0 and synphot 1.0. The problem is just if you end up with astropy 5.0 and synphot 1.0, which I think is the case you have.

Russell I expect this is frustrating for you, but in the big picture I would strongly advocate -not- letting these local dependency setup issues be a roadblock for merging the remaining PRs into WebbPSF develop. Honestly this is all going to be easier and will work more cleanly once everything is merged into a consistent branch so we're not dealing with as m many various ad hoc cases.

[Russell-Ryan]

No, it’s no problem. I just only speak GitHub at a second-grade level, and so I’ll just need gentle (and explicit) guidance when it comes to anything more complicated than a simple git clone and pip install.

Just an update on my end.

1. I was indeed running astropy 5.0 and synphot 1.1.0, they were the native installs to the pip install on webbpsf. For the time being, can we make the webbpsf setup.cfg point to explicit versions of astropy and synphot? Or maybe that’s not the right thought process, so ignore me if I'm off base.

2. I updated synphot with pip install synphot==1.1.1, and then all the problems went away with webbpsf/poppy. However, Charles jupyter notebook gave some strange errors, which I was able to resolve by just using my stand-alone python scripts. In short, it looked like jupyter didn’t understand that a helper function (to drive the multiprocessing pools) was not defined:

AttributeError: Can't get attribute '_fwhm_diagonal' on <module 'main' (built-in)>

I suspect this may be related to something goofy in my jupyter setup, so I don’t think this is a big deal for webbpsf/poppy.

3. Not really frustrating, I think I agree with you on the not letting it get in the way. The problem was, when I did some quick test for Justin, everything broke. And to compound matters, the astropy/synphot codes were released in the window of time that Andrea and I started and Justin and I changed things.

On Dec 6, 2021, at 11:45 AM, Marshall Perrin @.@.>> wrote:

OK, that error is due to a bug in synphot, in which prior versions are not compatibly with astropy 5.0. The required fix is to have the most recent synphot, version 1.1.1, if you have astropy 5.0 installed. This is an upstream bug and is not something we can fix within WebbPSF or poppy. Try updating your synphot.

This is a very recent problem - astropy 5.0 was only released a couple weeks ago, and the synphot 1.1.1 fix a few days later. In the case of WebbPSF we can't strictly require astropy 5.0 or synphot 1.1.1 because those are literally just weeks old, and we to choose support not just that bleeding edge; we have to also support older versions of code from ~1 year ago, in particular astropy 4.0 and synphot 1.0. The problem is just if you end up with astropy 5.0 and synphot 1.0, which I think is the case you have.

Russell I expect this is frustrating for you, but in the big picture I would strongly advocate -not- letting these local dependency setup issues be a roadblock for merging the remaining PRs into WebbPSF develop. Honestly this is all going to be easier and will work more cleanly once everything is merged into a consistent branch so we're not dealing with as m many various ad hoc cases.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/spacetelescope/poppy/issues/464*issuecomment-986953896__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!kBW5rm_JfEIYdp5sw6xH0R0VS_MUrifdLSFx4Lo41m_30KpUXHdJCeVBQ38nRg$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ACGLBMEHHKIAJ42PFVZ4D23UPTSCPANCNFSM5HHEDCAA__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!kBW5rm_JfEIYdp5sw6xH0R0VS_MUrifdLSFx4Lo41m_30KpUXHdJCeXNKoNbpA$. Triage notifications on the go with GitHub Mobile for iOShttps://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!kBW5rm_JfEIYdp5sw6xH0R0VS_MUrifdLSFx4Lo41m_30KpUXHdJCeWLB_mzdA$ or Androidhttps://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign*3Dnotification-email*26utm_medium*3Demail*26utm_source*3Dgithub__;JSUlJSU!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!kBW5rm_JfEIYdp5sw6xH0R0VS_MUrifdLSFx4Lo41m_30KpUXHdJCeW9JPYE2A$.

[mperrin]

This is an old thread about dependency issues from 1.5 years ago, related to astropy & synphot versioning at that time. I expect this is no longer current and so am going to mark this issue closed.