Infrastructure improvements for power spectrum fitting #5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 5 commits
May 15, 2025 18:10
… tau and k_parallel
sjforeman
requested changes
May 19, 2025
| print(f"Directory {d} does not match expected format, rejecting") | ||
| continue | ||
|
|
||
| key = mo.group(1) | ||
| print(f"Processing directory: {d}") |
There was a problem hiding this comment.
Suggested change
| print(f"Processing directory: {d}") | |
| logger.debug(f"Processing directory: {d}") |
Let's move to using logger instead of print. (There's a mix of logger and print throughout fitstack, and we should eventually change all the other print statements to use logger for consistency.)
Comment on lines
+973
to
+975
| nu21: float = 1420.40575177, # MHz | ||
| z_eff: Optional[float] = None, # effective redshift | ||
| cs: float = 299792.458, # speed of light in km/s |
There was a problem hiding this comment.
Suggested change
| nu21: float = 1420.40575177, # MHz | |
| z_eff: Optional[float] = None, # effective redshift | |
| cs: float = 299792.458, # speed of light in km/s | |
| z_eff: Optional[float] = None, # effective redshift |
nu21 and the speed of light are defined in cora.util.units, so we should use those definitions.
Comment on lines
+979
to
+985
|
|
||
| # Set default z_eff if None | ||
| self.z_eff = z_eff if z_eff is not None else 1.0 | ||
| cosmo_cora = cosmology.Cosmology() | ||
| self.H_z = cosmo_cora.H(self.z_eff) * u.mega_parsec / 1000. # In km/s/Mpc | ||
| self.nu21 = nu21 | ||
| self.cs = cs |
There was a problem hiding this comment.
Suggested change
| # Set default z_eff if None | |
| self.z_eff = z_eff if z_eff is not None else 1.0 | |
| cosmo_cora = cosmology.Cosmology() | |
| self.H_z = cosmo_cora.H(self.z_eff) * u.mega_parsec / 1000. # In km/s/Mpc | |
| self.nu21 = nu21 | |
| self.cs = cs | |
| # Set default z_eff if None | |
| self.z_eff = z_eff if z_eff is not None else 1.0 | |
| cosmo_cora = cosmology.Cosmology() | |
| self.H_z = cosmo_cora.H(self.z_eff) * u.mega_parsec / 1000. # In km/s/Mpc |
Remove local values of nu21 and cs
| Conversion factor | ||
| """ | ||
|
|
||
| C = (-1.0 / (2 * np.pi * self.nu21)) * (self.cs / self.H_z) * (1 + self.z_eff)**2 |
There was a problem hiding this comment.
Suggested change
| C = (-1.0 / (2 * np.pi * self.nu21)) * (self.cs / self.H_z) * (1 + self.z_eff)**2 | |
| C = (-1.0 / (2 * np.pi * u.nu21)) * ((u.c / u.kilo) / self.H_z) * (1 + self.z_eff)**2 |
Use cora's nu21 and speed of light.
Also, let's add a comment about the assumed units of k_parallel and tau that this factor is converting between.
| @@ -1101,7 +1140,9 @@ def multiply_pre_noncomp(self, signal: np.ndarray, **kwargs) -> np.ndarray: | |||
| signal : np.ndarray[npol, nkpar, nkperp] | |||
| The 2d power spectrum after multiplication with the relative FoG kernel. | |||
| """ | |||
|
|
|||
| # Calculate the conversion factor | |||
There was a problem hiding this comment.
Suggested change
| # Calculate the conversion factor | |
| # Calculate the conversion factor that converts from k_parallel to tau |
Comment on lines
+1164
to
+1165
| signal *= (1.0 + (scale0 * C * self.kpar[np.newaxis, :, np.newaxis]) ** 2) / ( | ||
| 1.0 + (scale * C * self.kpar[np.newaxis, :, np.newaxis]) ** 2 |
There was a problem hiding this comment.
It would also be useful to have a comment here about the units of kpar and scale0
| @@ -424,7 +424,16 @@ def average_data(cnt, pol=None, combine=True, sort=True): | |||
| distributed=False, | |||
| ) | |||
| avg.ps2D[:] = np.mean(darr, axis=0) | |||
| avg.ps2D_weight[:] = tools.invert_no_zero(np.var(darr, axis=0)) | |||
| #avg.ps2D_weight[:] = tools.invert_no_zero(np.var(darr, axis=0)) | |||
There was a problem hiding this comment.
Suggested change
| #avg.ps2D_weight[:] = tools.invert_no_zero(np.var(darr, axis=0)) |
Remove unneeded comment
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.