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.

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

Introduce experimental BasisCommonGP2 and FourierBasisCommonGP2 that accept selections #290

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from

Conversation

vallis
Copy link
Collaborator

@vallis vallis commented Aug 30, 2021

For instance, to implement independent correlated GPs on each telescope, one could do

rn = gp_signals.FourierBasisCommonGP2(pl, orf, selection=Selection(selections.by_telescope), Tspan=Tspan)

Note that it is currently necessary to set Tspan.

The PR also adds the method telescope() to pulsar, and by_telescope() to selections.

Michele Vallisneri added 4 commits August 30, 2021 11:22
FourierBasisCommonGP and BasisCommonGP2 that support selections
(e.g., to have separate common monopole GPs by telescope).
These require specifying Tspan manually.
@codecov
Copy link

codecov bot commented Aug 30, 2021

Codecov Report

Merging #290 (72360db) into master (f2b00be) will increase coverage by 0.35%.
The diff coverage is 96.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #290      +/-   ##
==========================================
+ Coverage   88.05%   88.41%   +0.35%     
==========================================
  Files          13       13              
  Lines        2980     3055      +75     
==========================================
+ Hits         2624     2701      +77     
+ Misses        356      354       -2     
Impacted Files Coverage Δ
enterprise/signals/gp_signals.py 91.23% <96.34%> (+0.92%) ⬆️
enterprise/signals/signal_base.py 90.65% <100.00%> (+0.54%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f2b00be...72360db. Read the comment docs.

@vhaasteren vhaasteren self-requested a review November 6, 2023 12:19
@vhaasteren
Copy link
Member

Hi @vallis , is this PR ready for review? I can properly look at it this week.

@vhaasteren vhaasteren changed the base branch from master to dev November 7, 2023 09:03
@vhaasteren
Copy link
Member

One question: is this a separate class instead of just updating BasisCommonGP, because this is experimental? Ideally the selections would just work for the regular common Common GP classes

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