Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cov_model and known_cov now parsed #332

Closed
wants to merge 2 commits into from
Closed

cov_model and known_cov now parsed #332

wants to merge 2 commits into from

Conversation

adeliegorce
Copy link
Contributor

Resolves #327

  • Added cos_model and known_cov arguments to functions related to data covariance in order to properly parsed the potential user-input covariance matrix/dictionary. These include: iC, cov_q_hat, q_hat, get_G, get_H, get_unnormed_V, get_analytic_covariance, scalar_delay_adujstement.
  • Added these parameters to each call on the above-listed functions. It clutters the code a little bit (also the docstrings), maybe we could consider making the cov_model and the known_cov dictionary attributes?
  • Added checks at the start of pspec function: asserts if cov_model is a string and if known_cov is given, in case the str isn't among the existing models (['empirical', 'dsets', 'autos', 'foreground_dependent']); if known_cov is given, asserts if the input_data_weight is set to something else than 'identity' - if it is set to identity, then raise warning saying that the input covariance will not be used in the R matrix.

Tested the code with the identity matrix as data covariance.

@adeliegorce
Copy link
Contributor Author

Running the tests highlighted issues in parsing the covariance information (even for already existing options such as 'dsets' and 'autos'): if input_data_weighting is set to 'iC', for any input cov_model, it is always the empirical covariance that will be computed, therefore the R matrix does not change with cov_model. In particular, it needs to depend on the time_index and so the format of Rkeys needs to be changed.

@adeliegorce adeliegorce closed this Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
continuous integration in progress
Projects
None yet
Milestone
Pleasant covariance matrix handling
Development

Successfully merging this pull request may close these issues.

User-input covariance not properly parsed to other functions
1 participant
@adeliegorce