search

litebird / litebird_sim

Simulation tools for LiteBIRD
GNU General Public License v3.0
18 stars 13 forks source link

Data splits in destriper #309

Closed ggalloni closed 1 month ago

ggalloni commented 4 months ago

This PR is meant to implement the same data splits of the binner (see #291), but in the destriper.

ggalloni commented 4 months ago

In this last commit, both binned and destriped maps out of the destriper result seem to be working.

Currently, only the single-split pipeline is in place, thus the sim.make_destriped_map.

ggalloni commented 4 months ago

The loop over every split is implemented. However, in the current implementation, each split will search for its baselines.

It would be useful to give the possibility to "recycle" the baselines between splits.

ggalloni commented 3 months ago

I added the recycling of baselines from the full splits.

Apart from propagating the relevant attributes and changes, the main modification is that if baselines are passed to 'make_destriped_map' it does not iterate to find a new solution and instead uses the provided one. Also, I "stored" the info about the solution being recycled in the 'converged' attribute of the 'DestriperResult' object. Now, that can also be a string (not only a bool as before). This string says to the user that the baselines have been recycled from the full split and also reports the convergence of the original run. This might not be the best solution, let me know what you think eventually.

A "side-effect" of this modification is that now the user can pass a custom baseline solution obtained externally.

ggalloni commented 2 months ago

I added a notebook doing some examples of data splits, for Binner and Destriper.

If you can, have a look at its flow and let me know if it is OK.

After this, I think we can proceed with merging.

ggalloni commented 2 months ago

Hi @ziotom78, I think this PR is mergeable if you agree.

Some tests are failing because apparently TOAST is breaking on macos, but I doubt that depends on this PR...

I discussed the interplay of this PR with #319 with @paganol and the best thing to do seems to be: we merge this into the master and then we merge again the master in #319 before starting working on the destriper.

What do you think?

ziotom78 commented 2 months ago

Hi @ziotom78, I think this PR is mergeable if you agree.

Some tests are failing because apparently TOAST is breaking on macos, but I doubt that depends on this PR...

I discussed the interplay of this PR with #319 with @paganol and the best thing to do seems to be: we merge this into the master and then we merge again the master in #319 before starting working on the destriper.

What do you think?

Great! I agree, let's do this.