Ldev#101
Conversation
😀 I would not worry about the unit and integration tests, probably OK to ignore/deactivate them for now I suggest @carronj to review the PR, not using the opfilts and cs_iterator myself anymore |
|
There was a problem hiding this comment.
Pull request overview
This PR implements a split iterative estimator for CMB lensing reconstruction with extensive enhancements to support the split estimator workflow, including memory optimization options and expanded bias computation capabilities.
Key Changes
- Addition of split iterative estimator functionality in
cs_iterator.pywith optional caching to save disk space - New
degrademethods in MAP opfilt classes for multigrid preconditioner support - Expanded
cpp_sims.pywith comprehensive options for split estimator analysis - New cleaner implementation of iterative bias computation in
iterbiases_clean.py
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 34 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Added healpy as a required dependency |
| delensalot/utils.py | Added dls2cls_grad function for CAMB gradient cls conversion |
| delensalot/utility/utils_plot.py | Added p2k function for phi to kappa conversion |
| delensalot/utility/utils_hp.py | Updated alm_splice to use new Alm.getlmax signature |
| delensalot/utility/cpp_sims.py | Major refactoring with split estimator support, memory management, and expanded analysis methods |
| delensalot/core/opfilt/*.py | Added degrade methods to multiple opfilt classes for preconditioner support |
| delensalot/core/iterator/statics.py | Enhanced is_iter_done to check cached plm files |
| delensalot/core/iterator/cs_iterator.py | Implemented iterator_splitlik_cstmf class and memory/disk caching options |
| delensalot/core/cg/multigrid.py | Added no-lensing preconditioner options and improved logging |
| delensalot/core/cg/cd_solve.py | Added documentation comments explaining CG algorithm |
| delensalot/biases/rdn0_cs.py | Added compute_ss_ds_cross function for split estimator RDN0 |
| delensalot/biases/iterbiases_clean.py | New clean implementation of iterative bias computation with comprehensive documentation |
| delensalot/biases/iterbiasesN0N1.py | Enhanced with gradient cls support and lmax filtering |
Comments suppressed due to low confidence (2)
delensalot/utility/cpp_sims.py:1021
- Keyword argument 'version' is not a supported parameter name of method cpp_sims_lib.get_mf0.
plm_mf1_qe = self.get_mf0([idx, plm_shuffle(idx)], mf_sims=qe_mf_sims_1, version=self.version)
delensalot/utility/cpp_sims.py:1022
- Keyword argument 'version' is not a supported parameter name of method cpp_sims_lib.get_mf0.
plm_mf2_qe = self.get_mf0([idx, plm_shuffle(idx)], mf_sims=qe_mf_sims_2, version=self.version)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mf_noise_phas: phases of noise maps for the MF estimate | ||
| shift_phas: if False, all MF are estimated with the same pahse at all iterations, | ||
| if Turem, changes the phases used for MF estimate between iterations | ||
| if True, changes the phases used for MF estimate between iterations |
There was a problem hiding this comment.
Typo: "Turem" should be "True".
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
|
||
| def get_sim_plm(self, idx): | ||
| """Returns the input plm, depening if it is a sims_ffp10, a npipe sim or other sim""" | ||
| """Returns the input plm, depending if it is a sims_ffp10, a npipe sim or other sim""" |
There was a problem hiding this comment.
Typo: "depening" should be "depending".
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| cls_plen_fid['pp'] = cldd_fid * utils.cli(np.arange(len(cldd_fid)) ** 2 * np.arange(1, len(cldd_fid) + 1, dtype=float) ** 2 / (2. * np.pi)) | ||
| if 'wE' in version and it>0: | ||
| # Need to convert the template of the lensing power spectrum: Cldd*rho, into the reidual lensing of the map: Cldd*(1-rho) | ||
| # Need to convert the template of the lensing power spectrum: Cldd*rho, into the residual lensing of the map: Cldd*(1-rho) |
There was a problem hiding this comment.
Typo: "reidual" should be "residual".
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
I re-implemented my own filters and the "iterator" a while ago, mostly to support the joint reconstruction that I am working on |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@louisl3grand I've opened a new pull request, #102, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@louisl3grand I've opened a new pull request, #103, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@louisl3grand I've opened a new pull request, #104, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
…nts (#103) * Initial plan * Fix typo: change 'reidual' to 'residual' Co-authored-by: louisl3grand <50632020+louisl3grand@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: louisl3grand <50632020+louisl3grand@users.noreply.github.com>
Updates, including: