[BUG] Specviz: Model Parameters Failing

[duytnguyendtn]

Describe the bug
When trying to export model parameters in Specviz, I run into the following traceback:

>>> specviz.get_model_parameters(model_label="Model")
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/var/folders/93/k31j59sj2493rzfqxc8r5z0r000171/T/ipykernel_52724/2990741164.py in <module>
      1 # Retrieve the parameters from which our model was constructed:
----> 2 specviz.get_model_parameters(model_label="Model")

~/duydir/gitrepos/aas239-jwebbinar/envdryrun/lib/python3.8/site-packages/jdaviz/core/helpers.py in get_model_parameters(self, models, model_label, x, y)
    249         for key in parameters_cube:
    250             for param_name in parameters_cube[key]:
--> 251                 parameters_cube[key][param_name] = u.Quantity(
    252                     parameters_cube[key][param_name],
    253                     param_units[key].get(param_name, None))

~/duydir/gitrepos/aas239-jwebbinar/envdryrun/lib/python3.8/site-packages/astropy/units/quantity.py in __new__(cls, value, unit, dtype, copy, order, subok, ndmin)
    412         if unit is not None:
    413             # convert unit first, to avoid multiple string->unit conversions
--> 414             unit = Unit(unit)
    415 
    416         # optimize speed for Quantity with no dtype given, copy=False

~/duydir/gitrepos/aas239-jwebbinar/envdryrun/lib/python3.8/site-packages/astropy/units/core.py in __call__(self, s, represents, format, namespace, doc, parse_strict)
   2063 
   2064         else:
-> 2065             raise TypeError(f"{s} can not be converted to a Unit")
   2066 
   2067 

TypeError: {} can not be converted to a Unit

To Reproduce
Steps to reproduce the behavior:

1. Fit a model (in this case, a gaussian + constant model)
2. Manually edit a few parameters (amplitude, stdev) (not sure if this is related?)
3. Refit the model
4. Call get_model_parameters

Expected behavior
A dictionary of parameters should be returned, no tracdback

Screenshots

Desktop (please complete the following information):

• OS: macOS Catalina
• Browser: Chrome 95.0.4638.69
• Jupyter:
  jupyter core : 4.6.3
  jupyter-notebook : 6.0.3
  qtconsole : 4.7.4
  ipython : 7.15.0
  ipykernel : 5.3.0
  jupyter client : 5.3.4
  jupyter lab : not installed
  nbconvert : 5.6.1
  ipywidgets : 7.5.1
  nbformat : 5.0.6
  traitlets : 4.3.3

Package versions (please complete the following information):

appnope==0.1.0
asdf==2.6.0
asteval==0.9.18
astropy==4.0.1.post1
async-generator==1.10
attrs==19.3.0
backcall==0.1.0
bleach==3.1.5
Bottleneck==1.3.2
bqplot==0.12.12
bqplot-image-gl==0.5.0
certifi==2020.6.20
chardet==3.0.4
click==7.1.2
cycler==0.10.0
decorator==4.4.2
defusedxml==0.6.0
dill==0.3.1.1
echo==0.5.dev2+g982cb0b
entrypoints==0.3
fast-histogram==0.9
glue-astronomy==0.1.dev71+gebc0737
glue-core==0.16.0.dev306+g7fcd8283
glue-jupyter==0.2.dev158+gac55d30
glue-vispy-viewers==0.12.2
gwcs==0.13.0
h5py==2.10.0
idna==2.9
ipydatawidgets==4.0.1
ipygoldenlayout==0.3.0
ipykernel==5.3.0
ipympl==0.5.6
ipysplitpanes==0.1.0
ipython==7.15.0
ipython-genutils==0.2.0
ipyvolume==0.5.2
ipyvue==1.3.2
ipyvuetify==1.4.0
ipywebrtc==0.5.0
ipywidgets==7.5.1
jdaviz==0.1.dev415+g559ed1e
jedi==0.17.0
Jinja2==2.11.2
jsonschema==3.2.0
jupyter-client==5.3.4
jupyter-core==4.6.3
jupyter-server==0.1.1
jupyterlab-pygments==0.1.1
kiwisolver==1.2.0
MarkupSafe==1.1.1
matplotlib==3.2.1
mistune==0.8.4
mpl-scatter-density==0.6
nbconvert==5.6.1
nbformat==5.0.6
notebook==6.0.3
numpy==1.18.5
packaging==20.4
pandas==1.0.4
pandocfilters==1.4.2
parso==0.7.0
pexpect==4.8.0
pickleshare==0.7.5
Pillow==7.1.2
prometheus-client==0.8.0
prompt-toolkit==3.0.5
ptyprocess==0.6.0
Pygments==2.6.1
PyOpenGL==3.1.5
pyparsing==2.4.7
pyrsistent==0.16.0
python-dateutil==2.8.1
pythreejs==2.2.0
pytz==2020.1
PyYAML==5.3.1
pyzmq==19.0.1
qtconsole==4.7.4
QtPy==1.9.0
radio-beam==0.3.2
regions==0.4
requests==2.23.0
scipy==1.4.1
semantic-version==2.8.5
Send2Trash==1.5.0
six==1.15.0
spectral-cube==0.4.5
specutils==1.0
terminado==0.8.3
testpath==0.4.4
tornado==6.0.4
traitlets==4.3.3
traittypes==0.2.1
urllib3==1.25.9
voila==0.1.21
wcwidth==0.2.3
webencodings==0.5.1
widgetsnbextension==3.5.1
xlrd==1.2.0

Additional context (e.g. data files)
Using data from Cami's Redshift JDAT Notebook

[pllim]

I think the way param_units is being constructed is not compatible with compound model? I can reproduce this with the Specviz example notebook even without adjusting the default model parameters.

This is the state of the variables at the crash.

parameters_cube = {'Model': {'amplitude_0': -30.636646708160153, 'mean_0': 7896.582862865359, 'stddev_0': 335.8367122151644, 'amplitude_1': 60.98315677037903}}

param_units = {'Model': {'amplitude_0': {}, 'mean_0': {'x': Unit("Angstrom")}, 'stddev_0': {'x': Unit("Angstrom")}, 'amplitude_1': {}}}

[pllim]

Without knowing the original intent of this code, I am not sure how to fix it, but the problem is in this logic block, it blindly assigns .return_units or .input_units for some parameters:

jdaviz/jdaviz/core/helpers.py

Lines 241 to 244 in e338e9b

	if "amplitude" in name:
	param_units[model_name][name] = models[model_name].return_units
	elif "mean" in name or "stddev" in name:
	param_units[model_name][name] = models[model_name].input_units

This affects Gaussian1D in particular. For your use case:

>>> specviz.fitted_models['Model']
<CompoundModel(amplitude_0=32.06859862 1e-17 erg / (Angstrom cm2 s), mean_0=6932.22938107 Angstrom, stddev_0=0. Angstrom, amplitude_1=62.74025286 1e-17 erg / (Angstrom cm2 s))>
>>> specviz.fitted_models['Model'].input_units
{'x': Unit("Angstrom")}
>>> specviz.fitted_models['Model'].return_units
{}

[pllim]

This fixes the crash but the result isn't quite I would expect.

--- a/jdaviz/core/helpers.py
+++ b/jdaviz/core/helpers.py
@@ -248,8 +248,13 @@ class ConfigHelper(HubListener):
         # param_units[key][param_name]
         for key in parameters_cube:
             for param_name in parameters_cube[key]:
+                param_unit = param_units[key].get(param_name, None)
+                if isinstance(param_unit, dict):
+                    if key in param_unit:
+                        param_unit = param_unit[key]
+                    else:
+                        param_unit = None
                 parameters_cube[key][param_name] = u.Quantity(
-                    parameters_cube[key][param_name],
-                    param_units[key].get(param_name, None))
+                    parameters_cube[key][param_name], param_unit)

         return parameters_cube

{'Model': {'amplitude_0': <Quantity 29.55142238>,
           'mean_0': <Quantity 6858.38013446>,
           'stddev_0': <Quantity 63.44864852>,
           'amplitude_1': <Quantity 54.69296536>}}

[pllim]

I think getting rid of the special logic block for Gaussian parameters might be a more correct fix. I will submit a PR (see #976).