Add 4pi beam convolution

[mreineck]

This aims to add 4pi convolution with non-axisymmetric beams (very similar to the "conviqt" functionality in TOAST, but more accurate and hopefully also quicker).
Currently this cannot be fully implemented because the code requires

• a_lm of the sky model maps (probably solved by Alms in Mbs #306, I need to check how I can access them).
• a_lm of the beams. Those will be in the IMO at some point, but have not yet been integrated.

It should be possible to load the beams from files for testing purposes, using healpy.read_alm, but I'm not sure how to provide the corresponding file names in the input parameters and how they would be passed to the beam convolution module.
Unfortunately I'm not very familiar with the simulation framework and the really large set of tools it uses. I'm happy to help tweaking and debugging the underlying algorithms, but I need help from experts with the high-level integration.

[mreineck]

I must say that some of the pre-commit hooks are driving me quite crazy ...
I attempt to add the following lines to litebid_sim/__init__.py:

from .beam_convolution import (
    add_convolved_sky_to_observations,
)

This is what I'm trying to do:

(lbsim) martin@marvin:~/codes/litebird_sim$ git status
On branch beam_comvolution_2
Your branch is up to date with 'origin/beam_comvolution_2'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   litebird_sim/__init__.py

no changes added to commit (use "git add" and/or "git commit -a")
(lbsim) martin@marvin:~/codes/litebird_sim$ git diff
diff --git a/litebird_sim/__init__.py b/litebird_sim/__init__.py
index b4a8aa6..1985a0c 100644
--- a/litebird_sim/__init__.py
+++ b/litebird_sim/__init__.py
@@ -150,6 +150,9 @@ from .spacecraft import (
     SpacecraftOrbit,
     SpacecraftPositionAndVelocity,
 )
+from .beam_convolution import (
+    add_convolved_sky_to_observations,
+)
 from .version import __author__, __version__
 
 # Check if the TOAST2 mapmaker is available

So I have the mentioned changes in my tree.
Now I run

(lbsim) martin@marvin:~/codes/litebird_sim$ git commit -a -m "add method to __init__.py"
ruff.....................................................................Failed
- hook id: ruff
- files were modified by this hook

Found 1 error (1 fixed, 0 remaining).

ruff-format..............................................................Passed

If I now look at the status, all changes are gone!

(lbsim) martin@marvin:~/codes/litebird_sim$ git status
On branch beam_comvolution_2
Your branch is up to date with 'origin/beam_comvolution_2'.

nothing to commit, working tree clean

As you can see, the pre-commit hooks somehow remove my changes from __init__.py! What is going wrong here?

[paganol]

Hi @mreineck, I can help in the high level integration. I can work on this branch and prepare a coherent lbs interface, what do you think?
Regarding the pre-commit, I cannot help, I never had this kind of problem

[mreineck]

Thank you so much, @paganol, this is a great relief for me!

Concerning the precommit hook, I managed to get the crucial information of of ruff by ading -v:

(lbsim) martin@marvin:~/codes/litebird_sim$ ruff litebird_sim/__init__.py 
litebird_sim/__init__.py:22:5: F401 [*] `.beam_convolution.add_convolved_sky_to_observations` imported but unused
Found 1 error.
[*] 1 fixable with the `--fix` option.

So I need to add the new function to __all__ further down in the file. Makes a lot of sense, but I really don't feel that silently removing the import was helpful to understand the problem. Maybe we shouldn't use --fix and just let ruff complain about things (and explain them), until everything has been fixed manually.

[mreineck]

I wonder whether we should merge #337 wit this PR, then we can add the signal modulation by the HWP as well (optionally, of course!). The only additional required quantity for this is the time-dependent HWP angle.

[mreineck]

I know that the PR doesn't look like much, but apart from obtaining the missing input data (which, I admit, may be complicated to achieve!), it is pretty much complete. All the numerical details are handled by ducc.

[mreineck]

Thank you, @paganol, your interfacing work looks great to me!
I see that some error messages still refer to scan_map_in_observations; I'm happy to update them, unless there is some deeper connection to scan_map_in_observations that I don't see at the moment.
I'll also copy over mueller_convolver.py from #337 and close #337 afterwards; having these two changes in a single PR makes much more sense!

[paganol]

Hi @mreineck, sorry I forgot to update the error messages! No deeper connection 😅... Of course if we find better names for the lower level functions we can change them. About merging the PRs, I totally agree with you.

[mreineck]

Stupid question: is there a place in litebird_sim where I can put mueller_convolver.py, so that it is not reformatted by ruff? Strictly speaking this piece of code is maintained somewhere else (although it doesn't have an "official" place yet), and reformatting (and especially the re-naming of variables that ruff requests) will be quite a maintenance headache.
Should we have an external/ subdirectory?

[ziotom78]

Stupid question: is there a place in litebird_sim where I can put mueller_convolver.py, so that it is not reformatted by ruff? Strictly speaking this piece of code is maintained somewhere else (although it doesn't have an "official" place yet), and reformatting (and especially the re-naming of variables that ruff requests) will be quite a maintenance headache. Should we have an external/ subdirectory?

Would #fmt: off at the beginning of the file be ok, or do you want the file to be left completely untouched?

[mreineck]

No, that's perfectly fine, thanks for pointing this out!

[paganol]

Hi @mreineck, for matching the latest changes in the pointing API (PR #340) this line

litebird_sim/litebird_sim/beam_convolution.py

Line 65 in b671b68

curr_pointings_det = curr_pointings_det.reshape(-1, 3)

should be removed.

[mreineck]

Thanks for te tip, I'll fix this!
I'm travelling right now, but I hope to get back to this PR next week.