Tutorial Error when including shift

[lili-alderson]

When adding sh=value (rather than sh=None) in model=marg.Transit(), y_smooth_model = model.calc(pars=params, x=x_smooth) creates the following error. Both the data and the shift have the same length

ValueError                                Traceback (most recent call last)
<ipython-input-221-2a622eda2227> in <module>
      8 
      9 # Calculate model on denser grid to display
---> 10 y_smooth_model = model.calc(pars=params, x=x_smooth)

~/Documents/ExoTiCISM/ExoTiC-ISM/exoticism/margmodule.py in calc(self, pars, x, *args, **kwargs)
    144     def calc(self, pars, x, *args, **kwargs):
    145         """Evaluate the model"""
--> 146         return _transit_model(pars, x, self.sh_array, x_in_phase=self.x_in_phase)
    147 
    148 

~/Documents/ExoTiCISM/ExoTiC-ISM/exoticism/margmodule.py in _transit_model(pars, x, sh, x_in_phase)
     83     mulimb0, mulimbf = occultnl(rl, c1, c2, c3, c4, b0)
     84     systematic_model = sys_model(phase, HSTphase, sh, m_fac, hstp1, hstp2, hstp3, hstp4,
---> 85                                  xshift1, xshift2, xshift3, xshift4)
     86 
     87     # model fit to data = transit model * baseline flux (flux0) * systematic model

~/Documents/ExoTiCISM/ExoTiC-ISM/exoticism/margmodule.py in sys_model(phase, hst_phase, sh, m_fac, hstp1, hstp2, hstp3, hstp4, xshift1, xshift2, xshift3, xshift4)
    425     sys_m = (phase * m_fac + 1.0) * (
    426             hst_phase * hstp1 + hst_phase ** 2. * hstp2 + hst_phase ** 3. * hstp3 + hst_phase ** 4. * hstp4 + 1.0) * (
--> 427                     sh * xshift1 + sh ** 2. * xshift2 + sh ** 3. * xshift3 + sh ** 4. * xshift4 + 1.0)
    428 
    429     return sys_m

~/opt/miniconda3/envs/exoticism/lib/python3.7/site-packages/astropy/units/quantity.py in __mul__(self, other)
    941                 return NotImplemented
    942 
--> 943         return super().__mul__(other)
    944 
    945     def __imul__(self, other):

~/opt/miniconda3/envs/exoticism/lib/python3.7/site-packages/astropy/units/quantity.py in __array_ufunc__(self, function, method, *inputs, **kwargs)
    475 
    476         # Call our superclass's __array_ufunc__
--> 477         result = super().__array_ufunc__(function, method, *arrays, **kwargs)
    478         # If unit is None, a plain array is expected (e.g., comparisons), which
    479         # means we're done.

ValueError: operands could not be broadcast together with shapes (2197,) (52,)

[lili-alderson]

The problem arised because we have the line
x_smooth = np.arange(data_x[0], data_x[-1], CONFIG_INI.getfloat('smooth_model', 'resolution')) (1)
Which turns the x values from having (in the case of W17) 52 values to having 2197 - therefore terms in systematics_model which are calculated from x (so things like hst_phase) end up having 2197 values, while anything involving the shifts is stuck with the original 52 (ie sh * xshift1 + sh ** 2. * xshift2 + sh ** 3. * xshift3 + sh ** 4. * xshift4 + 1.0), hence adding all the terms together doesn't work

The basic solution to this problem would be to create a similar line to (1) involving the shifts so it has the same length, but that doesn't make sense physically. x is a continuous dataset but the corresponding shift isn't

[hrwakeford]

Works when
sh=None and x_in_phase=True
sh=None and x_in_phase=False

Does not work when
sh=shift and x_in_phase=True
sh=shift and x_in_phase=False

This is because the sh array is being called somewhere in the calculation of the transit model which is then used to calculate the smooth model and as sh is not then transformed to the new x_smooth then it does not work with the module being called.

@ivalaginja could you take a look at this in the notebook you can change the parameters above on In[7]

model = marg.Transit(data_x[0], msmpr, c1s, c2s, c3s, c4s, flux0=data_y[0],
                     x_in_phase=True, name='transit_model', sh=None)

Then going down to In[11] with the line

# Calculate model on denser grid to display
y_smooth_model = model.calc(pars=params, x=x_smooth)

This model.calc seems to call transit model not just marg.occultnl and it then gets the error printed in the first comment.

This needs to be determined if it is just a tutorial notebook problem or if this is an issue in the main code.

[hrwakeford]

This is clearly limited to the tutorial notebook and the main margmodul and marginalization codes work well locally.

[ivalaginja]

Ah I see. Yes, I can have a look at this this week.

[ivalaginja]

Looking at this, I confirm what @hrwakeford said above: This is an issue in the notebook only, not in the main marginalisation code. This is because the tutorial tries to exploit as many of the internal Sherpa functionality as possible, since it presents a simplified example. The main code on the other hand still uses some own functions where Sherpa could be used, in this case the evaluation of the model on a smooth grid.

We can see in the main code (marginalisation.py) that in there, we do not in fact use the sherpa-internal method to calculate the smooth model. Instead, we use marg.occultnl() directly (nothing wrong with that, as it works well):
https://github.com/hrwakeford/ExoTiC-ISM/blob/c03ed7e1a375f6f5336987263b9da4addd1c664f/exoticism/marginalisation.py#L322
We actually have an issue open (#35) that aims to look into whether we can replace this with the Sherpa-internal method (model.calc()) - and by finding this bug in the notebook, we actually found a thing that would need to be worked on in order to make this possible, so this is good.

I will now look for a solution.

[ivalaginja]

The difference occurs because marg.occultnl() only calculates the Mandol & Agol transit model, while model.calc() uses the model evaluation function of Transit(), which is _transit_model(). And this one does not only calculate the analytic light curve with occultnl(), but it combines it with a systematic model:
https://github.com/hrwakeford/ExoTiC-ISM/blob/c03ed7e1a375f6f5336987263b9da4addd1c664f/exoticism/margmodule.py#L88

It is only the systematic model that needs the shift array sh.

[hrwakeford]

Yeah the problem in the tutorial notebook comes when you include your own gris of sh values and then follow the rest of the procedures. And it is in the calling of the model evaluator that is then essentially taking in more information than needed for the smooth model generation. I think this is fairly minor and just then needs some details at the top of the notebook to outline the situations where it is valid. So for example: This notebook is just a simple example to demonstrate how the package handles the input light curves. For example, it does not include fitting for positional shifts via the sh array to speed up the fitting process and provide visual examples of the routine.

[ivalaginja]

Yup, I see. Would you like me to do that, add clarifying notes in the notebook and call it? If it is not physical to include a grid of sh values when calculating a smooth model, then there is no point in hacking the code to enable it to do something of that sort.
I can also include error messages in the model itself that can be raised in case this is tried in the future, on top of the clarifications in the notebook. Would that make sense?

[hrwakeford]

Yeah it does not make sense to include the sh values when making the smooth model. As we have in the main code package.

I think add a note to the top of the notebook and I like the idea of the error message as well just so it is clear. As the main code calculates the smooth model in a different way I think we can leave it for now and close this issue.

I think what we might do @lili-alderson is think of putting together a more complete tutorial in the future if needed.

[ivalaginja]

Ok cool, I am on it. Yes, a more extensive tutorial going into more details would certainly be a great addition. And, it's good that @lili-alderson found this, as this will directly inform any potential work in issue #35.

[ivalaginja]

Resolved with #112.