Skip to content

Fix input sky unit error - #549

Open
mj-gomes wants to merge 3 commits into
masterfrom
fix_bandintegration_units
Open

Fix input sky unit error#549
mj-gomes wants to merge 3 commits into
masterfrom
fix_bandintegration_units

Conversation

@mj-gomes

@mj-gomes mj-gomes commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

This PR fixes #548

origin_unit in _get_cmb_unit_conversion() is by default K_CMB, but then when calling pysm3.bandpass_unit_conversion we do not specify that as the input unit, so it was using the default uK_RJ.

Now it is working.

TODO before merging: (DONE)

  • Fix expected values in tests

@mj-gomes
mj-gomes requested a review from paganol August 6, 2026 14:26
@mj-gomes mj-gomes self-assigned this Aug 6, 2026
@mj-gomes mj-gomes added the bug Something isn't working label Aug 6, 2026
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  litebird_sim
  input_sky.py
  mpi.py
  observations.py
  simulations.py
  litebird_sim/mapmaking
  common.py
  destriper.py
Project Total  

This report was generated by python-coverage-comment-action

@mj-gomes

mj-gomes commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Added a CI test for the verification of correct conversion when bandpass integrating.

@mj-gomes

Copy link
Copy Markdown
Contributor Author

I noticed something which can be improved for tidiness. Currently, we assume the input cmb power spectrum used to generate the cmb map is always in uK_CMB (see here), and then we apply the scaling factor so that our base units are K_CMB. We then perform conversions from that on, and that is coherent with the default unit in here being K_CMB.

For this reason, the following part of the cmb unit conversion code is never reached:

# If origin is not K_CMB, we calculate the factor for the origin unit
# and take the ratio.
# Value_Target = Value_K_CMB * C_target
# Value_Origin = Value_K_CMB * C_origin => Value_K_CMB = Value_Origin / C_origin
# Value_Target = (Value_Origin / C_origin) * C_target
# Factor = C_target / C_origin
factor_to_origin = pysm3.bandpass_unit_conversion(
freqs, weights, origin_astropy
)
return (factor_to_target / factor_to_origin).value

We should either delete this part, or relax the assumption that the input power spectrum is always in uK_CMB (and in that case add a parameter to SkyGenerator stating the unit of the input spectrum in case one is given).

What do you think? I suppose we always use uK_CMB, so I don't know if the second option is relevant, but in that case we can just delete the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unit problem with bandpass_integration in input sky generation

1 participant