Skip to content

Include the HEALPix pixel window in map-based pseudo-Cℓ bandpower windows - #366

Open
cailmdaley wants to merge 1 commit into
developfrom
fix/pseudo-cl-pixel-window
Open

cailmdaley wants to merge 1 commit into
developfrom
fix/pseudo-cl-pixel-window

Conversation

@cailmdaley

Copy link
Copy Markdown
Collaborator

For a spectrum measured on HEALPix maps, the bandpowers NaMaster returns are W·(pw²·C_ℓ): pixelisation smooths the field by the pixel window pw(ℓ), and the mode-coupling inversion does not undo it. The pseudo-Cℓ SACC stored W alone, so anything that predicts the data through the SACC window (a SACC likelihood, the Smokescreen Cℓ_EE shift in #253) compared the measurement to W·C_ℓ. This PR puts pw²(ℓ) into the stored window for map-based spectra.

What changes

  • bandpower_window_from_workspace(wsp, nside=None) multiplies W by hp.pixwin(nside)**2 when nside is given.
  • pseudo_cl_to_sacc and PseudoClMixin.pseudo_cl_to_sacc_part forward nside; calculate_pseudo_cl_map passes its map nside. The catalogue-based path passes none: it has no pixels, so its window is unchanged.

Effect on results

  • Workflow products: none. generate_pseudo_cl.py, generate_pseudo_cl_cov.py and papers/cosmo_val all use cell_method="catalog".
  • Map-based SACCs (cell_method="map", the CosmologyValidation default): at the configured nside = 1024, ℓ_max = 2·nside = 2048, pw² suppresses the prediction by 0.09% at ℓ = 100, 2.2% at 500, 8.5% at 1000 and 32% at 2047. The top powspace bands are the ones a fit through the old window got wrong.
  • The Gaussian-covariance fiducial in calculate_pseudo_cl_eb_cov already multiplies by pw², so the covariance and the window now agree.

Interaction with #253

The blinding shift is ΔCℓ_EE passed through the SACC window (PRD #241 §4). Once both land, the shift and the data it conceals use the same window, pw² included, for map-based spectra.

Verified

test_pseudo_cl_to_sacc_real_namaster checks the map-based window equals pw²(ℓ)·W against a real NaMaster workspace (rtol 1e-12). Container suite: 306 passed, 1 xfailed.

— Claude (Opus) on behalf of Cail.

🤖 Generated with Claude Code

bandpower_window_from_workspace(wsp, nside=) multiplies NaMaster's
decoupling window by pw²(ℓ) for a spectrum of HEALPix maps at nside, so
the SACC window maps a theory C_ℓ to the measured bandpower. The map-based
cosmo_val path passes its nside through pseudo_cl_to_sacc_part and
pseudo_cl_to_sacc; the catalogue-based path has no pixel window and passes
none.

With the blinding PR (#253) the Cℓ_EE shift is computed through the SACC
window as written. On its own that is consistent (data and shift share the
window); once both land, this fix corrects the data and the shift together.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@sachaguer sachaguer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is probably a big comment. I just realised this issue. But this PR and probably others is not sourced from the most recent code used to compute the pseudo Cl which is in the branch feature/sp_validation-extend-to-tomo. The API has changed quite a lot and we need to find a way to merge the SACC updates with this tomography code as early as possible.

What suprised me in this PR is that the function for the iNKA covariance has been renamed in the new API and the window function is not accounted for anymore. Generally speaking we should run across the previous PRs and make sure that they do not have the same problem otherwise it is going to be very difficult to merge.

@cailmdaley

Copy link
Copy Markdown
Collaborator Author

Thanks Sacha, good catch, and sorry for any carelessness on my part that led to this. What's your proposed order here? Merge the SACC updates into your feature branch while keeping its pseudo-Cl API, then merge that branch into develop before much further sp_validation development happens? We could also split off the API redesign from the full tomographic feature extension if that would be faster.

@cailmdaley

Copy link
Copy Markdown
Collaborator Author

Also please tell me which parts (if any) of the necessary work you feel comfortable delegating to Claude.

@sachaguer

Copy link
Copy Markdown
Contributor

Just added a comment along the same lines in another PR so let's keep the discussion on that matter here.

I think two important PRs are opened and target the feature branch sp_validation_extent-to-tomo. I believe this is my leakage extension to tomography which is pretty much done albeit the print functions that I need to finalise. The other one is Lisa's config space update.

We should aim for the easiest but I see two options:

  • Merge first the tomo extension in develop but we have to do something about the child opened PRs.
  • Redirect the SACC updates to the tomo extensions and propagate its modifications in this new API.

As I will not be programming these things myself actively on a short term I am happy if Claude takes charge of some of this. But a first step is to check if some of the fix opened PR are not superfluous and are not correcting for something I already fixed/removed. I think this is well documented in the PRs for Claude to understand.

@cailmdaley

Copy link
Copy Markdown
Collaborator Author

Great, that's very clear. I'll have claude check the open PRs for duplication. I don't quite understand the distinction between the two options you mention, since the SACC updates are already into develop. Does that mean that the only option is to merge the SACC updates in develop into feature/sp_validation-extend-to-tomo, then propagate to the child PRs?

@sachaguer

Copy link
Copy Markdown
Contributor

Maybe there is no difference because things are intertwined because the sacc updates and bug fix PRs touch both the harmonic space estimator and the real space ones. In the sp_validation-extent-to-tomo I believe that harmonic space and rho/tau are already merge so there will be a conflict to bring the feature branch to develop because of the sacc update. So we need to merge. But even if this is done, I believe API for the correlation functions might have changed as well and this is in a child PR that has not been merged yet to sp_validation-extent-to-tomo. So we have to go down the PRs and back up anyway.

@cailmdaley

Copy link
Copy Markdown
Collaborator Author

here is the proposed order we came up with: #375

basically i will merge #307 into feature/sp_validation-extend-to-tomography, then open a PR to merge develop into feature/sp_validation-extend-to-tomography which you and Lisa can review. meanwhile #253 can be independently reviewed and merged into develop, since it doesn't depend on the tomography work.

@LisaGoh wanted to give you a heads up about this development! have a nice weekend both

This branch has not been deployed

No deployments
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