Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data splits in destriper #309

Merged
merged 38 commits into from
Jun 6, 2024
Merged

Data splits in destriper #309

merged 38 commits into from
Jun 6, 2024

Conversation

ggalloni
Copy link
Collaborator

@ggalloni ggalloni commented Mar 21, 2024

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

  • Implement same splits of Splits in mapmaker #291
  • Fix tests
  • Add high-level function mimicking make_binned_map_splits
  • Add possibility to recycle baselines
  • Add documentation

I propagated the time and det splits to the `_compute_binned_map` function (and all the liked ones)
@ggalloni
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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 and others added 13 commits March 27, 2024 15:30
Now, the errors on baselines are not looped over detectors when saved, since they do not depend on them (they are summed over deterctors). The same is done for loading a `DestriperResult` from file.

See also #310 for more details.
Fix bug happening when no destriping is perfomed, but report is requested
When a list of splits is passed, we assume the computation of the full split first. This allows to recycle its baselines
@ggalloni
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

ggalloni commented Jun 5, 2024

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
Copy link
Member

ziotom78 commented Jun 6, 2024

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.

@ggalloni ggalloni merged commit ad9d56f into master Jun 6, 2024
5 of 9 checks passed
@paganol paganol deleted the splits_in_destriper branch November 14, 2024 08:58
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