-
Notifications
You must be signed in to change notification settings - Fork 2
Multi-constellation #4
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
base: master
Are you sure you want to change the base?
Conversation
…lass; draft for LISALIA detector
|
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. |
damonge
left a comment
There was a problem hiding this 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,LISADetector2andALIADetector2. Can you explain briefly what the motivation is for each of these? - When building a constellation, instead of creating new detector classes like
TwoLISADetectororMultipleLISADetector, would it have been possible to just concatenate detectors into a network connected with the appropriate noise correlation matrix?
schnell/correlation.py
Outdated
| static=det.detector.static, | ||
| include_GCN=det.detector.include_GCN, | ||
| mission_duration=det.detector.mission_duration) | ||
| LISA2det = LISADetector2(0, is_L5Gm=det.is_L5Gm, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
schnell/correlation.py
Outdated
| mission_duration=det.detector.mission_duration) | ||
| self.psdaL1 = LISA1det.psd_A | ||
| self.psdxL1 = LISA1det.psd_X | ||
| self.psdaL2 = LISA2det.psd_A |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
schnell/correlation.py
Outdated
| x = self.psdxL1(f) | ||
| return x/a | ||
|
|
||
| def _rhoL2(self, f): |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
schnell/correlation.py
Outdated
| mat[:, j, i] = rL2 | ||
| return mat | ||
|
|
||
| class NoiseCorrelationMultipleLISA(NoiseCorrelationBase): |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually no, indeed
schnell/detector.py
Outdated
| array_like: array of PSD values in units of \ | ||
| 1/Hz. | ||
| """ | ||
| return self.psd_A(f) + self.GCN(f) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
schnell/mapping.py
Outdated
| NoiseCorrelationConstantIdentity, | ||
| NoiseCorrelationConstantR) | ||
|
|
||
| from concurrent.futures import ProcessPoolExecutor as Pool |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
schnell/space_detector.py
Outdated
| import scipy.integrate as integr | ||
| from .detector import Detector, LISAlikeDetector | ||
|
|
||
| class LISADetector2(LISAlikeDetector): |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
schnell/space_detector.py
Outdated
|
|
||
| def _pos_single(self, t, n): | ||
| if n in [0, 1, 2]: | ||
| detect = LISADetector2(0, is_L5Gm=self.is_L5Gm, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
schnell/space_detector.py
Outdated
| for i in range(6)]) | ||
|
|
||
| def _pos_single(self, t, n): | ||
| detect = LISADetector2(0, is_L5Gm=self.is_L5Gm, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same question here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
…oiseCorrelationMultipleSpaceDetectors
@Alseidon 's modifications