Skip to content

Conversation

@damonge
Copy link
Owner

@damonge damonge commented Jun 7, 2021

@Alseidon 's modifications

@Alseidon
Copy link
Collaborator

Alseidon commented Jun 8, 2021

Not sure if test notebooks and npy_saves are to be added to the repo. Also, the new LISA and ALIA classes (using the LISAlike detector superclass) are named LISADetector2 and ALIADetector2 (for testing purposes). Should probably change this and delete the original classes.

Copy link
Owner Author

@damonge damonge left a comment

Choose a reason for hiding this comment

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

OK, thanks a lot @Alseidon I've left a few questions in comments below. Here are a couple of questions to understand better what you've been doing:

  • You have LISADetector, LISAlikeDetector, ALIADetector, LISADetector2 and ALIADetector2. Can you explain briefly what the motivation is for each of these?
  • When building a constellation, instead of creating new detector classes like TwoLISADetector or MultipleLISADetector, would it have been possible to just concatenate detectors into a network connected with the appropriate noise correlation matrix?

static=det.detector.static,
include_GCN=det.detector.include_GCN,
mission_duration=det.detector.mission_duration)
LISA2det = LISADetector2(0, is_L5Gm=det.is_L5Gm,
Copy link
Owner Author

Choose a reason for hiding this comment

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

Do you really need two copies of this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not at all, I will change this

mission_duration=det.detector.mission_duration)
self.psdaL1 = LISA1det.psd_A
self.psdxL1 = LISA1det.psd_X
self.psdaL2 = LISA2det.psd_A
Copy link
Owner Author

Choose a reason for hiding this comment

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

Or this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Same

x = self.psdxL1(f)
return x/a

def _rhoL2(self, f):
Copy link
Owner Author

Choose a reason for hiding this comment

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

Or this? Is this notthe same as _rhoL1?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Same

mat[:, j, i] = rL2
return mat

class NoiseCorrelationMultipleLISA(NoiseCorrelationBase):
Copy link
Owner Author

Choose a reason for hiding this comment

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

Do you need the TwoLISA case if you have this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually no, indeed

array_like: array of PSD values in units of \
1/Hz.
"""
return self.psd_A(f) + self.GCN(f)
Copy link
Owner Author

Choose a reason for hiding this comment

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

Any reason why this is not added in the psd_A method directly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I wasn't sure where to add it, if it makes more sense in psd_A then let's do this

NoiseCorrelationConstantIdentity,
NoiseCorrelationConstantR)

from concurrent.futures import ProcessPoolExecutor as Pool
Copy link
Owner Author

Choose a reason for hiding this comment

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

Did you need this in the end?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, should be deleted

return gls


def get_G_ell_part(self, t, f, nside, indices, no_autos=False, deltaOmega_norm=True,
Copy link
Owner Author

Choose a reason for hiding this comment

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

What are these two functions supposed to do exactly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

They just allowed me to compute a part of the sum over the detectors in Nell^-1 and were mainly for checking purposes ; maybe I should delete them

import scipy.integrate as integr
from .detector import Detector, LISAlikeDetector

class LISADetector2(LISAlikeDetector):
Copy link
Owner Author

Choose a reason for hiding this comment

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

So the difference wrt the default LISA is adding the GCN?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The default LISA was just there as a safeguard, in case the LISAlike superclass didn't work. It should be deleted now (and LISADetector2 renamed to LISADetector)
Same for ALIA


def _pos_single(self, t, n):
if n in [0, 1, 2]:
detect = LISADetector2(0, is_L5Gm=self.is_L5Gm,
Copy link
Owner Author

Choose a reason for hiding this comment

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

Humm, could you have used self.detector instead if you checked if n == self.i_d? Just wondering if having to define a new detector every time will slow things down.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure how much time we would gain, but it's really easy to do so I will probably implement this soon

for i in range(6)])

def _pos_single(self, t, n):
detect = LISADetector2(0, is_L5Gm=self.is_L5Gm,
Copy link
Owner Author

Choose a reason for hiding this comment

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

same question here

Copy link
Collaborator

Choose a reason for hiding this comment

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

same

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.

3 participants