Skip to content

Conversation

@jorgemarpa
Copy link
Contributor

@jorgemarpa jorgemarpa commented Apr 6, 2022

This PR adapts machine to use the new perturbation classes introduced in #59.

It modifies the following methods:

  • self.build_time_model()
  • self.plot_time_model()
  • self.fit_models()
  • new self._get_perturbed_model(), computes the perturbed matrix for a given time.

It adds a new attribute self.P3 which is a PerturbationMatrix3D object that manages the perturbation matrix.

TODO:

  • fix pytest
  • fix flake8
  • test focus component
  • test centroid components
  • check for attribute consistency

EDIT:
The original mac.time_corrector = "pos_corr", "centroid" options can be mimic by including the following arguments to PerturbationMatrix3D at initialization: other_vectors = [poscorr1, poscorr2, poscorr1 * poscorr2], poly_order=0. In this way the perturbation matrix will use only the centroids/poscorr as components with no time.

@jorgemarpa jorgemarpa force-pushed the machine-perturbation branch from 95d05bc to 5e2de68 Compare April 6, 2022 19:56
Copy link
Contributor Author

@jorgemarpa jorgemarpa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christinahedges I reviewed this PR again, added some minor docstrings, and removed duplicated functions in utils.py.

It looks Ok to me. Please review again before merging

@christinahedges christinahedges changed the title [WIP] Machine uses perturbation class Machine uses perturbation class Jun 21, 2022
Copy link
Contributor

@christinahedges christinahedges left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good, just two very minor docstring changes and then I will merge

def pca(self, y, ncomponents=5, smooth_time_scale=0):
"""Adds the principal components of `y` to the design matrix
Will add two time scales of principal components, definied by `long_time_scale`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't true anymore can we fix this docstring here?

pixels_in_tpf = np.ones_like(self.row, dtype=bool)

# enheance pixel mask
time_corr = np.nanpercentile(bkg_flux, 20, axis=1)[:, None]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this 20%? Is this why stuff goes negative so often? Can you add a short docstring explainer about what's going on here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That part is only used to create a naive model of the background as a function of time to then find and mask out variable pixels. So it does not play into building the bkg model.
Changing the percentile will only change which pixels to look at in the distribution each time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants