Derivatives for satellite_parallax don't work

[jenniferyee]

FitData.get_d_A_d_params_for_point_lens_model() does not currently take into account satellite_skycoords when calculating a model. This results in bad parallaxes.

This is caused by an underlying architecture issue. We have a single model defined for all datasets. Each instance of FitData created by Event uses that model and a single dataset. Then, we rely on FitData to access satellite_skycoords from the dataset when calculating the magnification. However, we need the underlying trajectory when calculating the derivatives of the parallax with respect to the model. This trajectory is accessed in many places. Right now, that trajectory is accessed using model.get_trajectory, which does not take satellite_skycoords as an argument. This information is only part of the model IF the model has ephemerides_file as an argument. However, this isn't required because we were relying on the dataset to provide this information.

1. [Potential immediate solution:] If we include the ephemerides_file as part of the model, will that break the calculation for ground-based data? Probably, and that solution wouldn't work in the case of multiple satellites.
2. [Semi-rhetorical:] How can we fix the underlying architecture? Is this part of the overhaul needed for Model classes in general?

[jenniferyee]

I have verified that adding ephemerides_file to Event.model results in equally broken behavior (because the ephemerides_file gets applied to the ground-based data as well).

[jenniferyee]

Proposed solution: Adapt code from mm.Model to create a new function

def get_dataset_trajectory(self):
    if self.dataset.ephemerides_file is None:
        return self.model.get_trajectory(self.dataset.time)
    else:
        kwargs_ = {
            'times': self.dataset.time, 'parallax': self.model._parallax,
            'coords': self.model.coords,
            'satellite_skycoord': self.dataset.satellite_skycoord}

        return mm.Trajectory(parameters=self.model.parameters, **kwargs_)

Then, any time FitData needs the trajectory for a particular dataset, instead of using self.model.get_trajectory, it would use self.get_dataset_trajectory(). I have implemented this in the finite_source_gradient branch. If people agree (@rpoleski), I can implement this change in master.

[rpoleski]

Please provide a code snippet that shows the problem.

[rpoleski]

I agree. You want to merge it to master, or copy-paste get_dataset_trajectory() to master branch?

[jenniferyee]

I was proposing to copy-paste get_dataset_trajectory() to master branch.

[jenniferyee]

This was implemented in commit 81abdfeba00dc0de6dbf5b5ddf4c91409db3d145. Added a crude unit test to verify implementation in commit fda28dcd9c2a047d09edc6e2f86f7a8a44882653 (checks that the results are different with and without an ephemerides file for the dataset).