Skip to content

Conversation

@Saladino93
Copy link
Contributor

Here I implement a version where I sample over the Gaussian field of a log-normal field. Which tests should I do?

The relevant files are:

cs_iterator_lognormal.py and the stepper.py

All the others can be ignored (foreground, iterative bh, etc...) as they are part of other branches.

@Saladino93 Saladino93 changed the title Odevln Lognormal sampling Aug 16, 2023
@Saladino93 Saladino93 added the enhancement New feature or request label Aug 16, 2023
Copy link
Member

@Sebastian-Belkner Sebastian-Belkner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR introduces two great new features to delensalot.

  • The first is a new iterator that finds the MAP point by assuming that phi is lognormally distributed. It also provides ways to generate lognormally distributed simulations that can be used for the reconstruction. This is in particular interesting for delensalot as we know that phi is not perfectly Gaussian. While the standard delensalot iterators work with non-gaussian phis by assuming it to be Gaussian, a reconstruction that supports non-gaussian phis is a great extension.

  • The second is an extension of the simulation library. The PR introduces the generation of point source foregrounds and is embedded in the already existing simulation library.

I am happy to approve it once the comments and merge conflict are resolved.

tlm = self.unlcmbs.get_sim_tlm(self.offset_index(idx, self.offset_cmb[0], self.offset_cmb[1]))
dlm = self.get_sim_plm(idx)

hp.write_alm(pfname, dlm)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could lead to unexpected behaviour. A function returning tlms shouldn't write dlms.


if (i + 1) in itrs:
ret.append(rlm2alm(rlm))
print("Doing for kappa0", kappa0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a huge print for each iteration. Suggest to use log.debug() instead


@staticmethod
def load_plms(lib_dir, itrs):
def load_plms(lib_dir, itrs, kappa0 = None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the kappa0 do? Docstrings would help

rlm2alm = lambda x : x.copy()


from plancklens import shts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plancklens.sht only part of master branch, delensalot however uses plancklens/plancklensdev. This line will therefore fail.

Suggesting to use lenspyx shts directly, as it is done throughout delensalot. Also, plancklens dependecy will be removed in the near future.


from . import cs_iterator as csit

alm2rlm = lambda alm : alm # get rid of this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they can be deleted


from plancklens import shts
from delensalot.utility.utils_hp import Alm, almxfl
from plancklens.utils import cli
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delensalot has its own utils.cli()

log.info("\r [" + ('%02d:%02d:%02d' % (dh, dm, ds)) + "] " + label)
return

typs = ['T', 'QU', 'TQU']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is used


def transform(rlm, kappa0):
if kappa0 is not None:
print('kappa0 is not None, transforming to phi')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggesting to use log.warning or log.debug instead, so there is context information and so that it can be controlled via verbosity

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation uses the pertmf iterator. Does this also work for the const_mf iterator?

The procedure of your implementation will be described in the n32 paper, but having the outline described in the docstring would be very helpful for future reference



class iterator_pertmf(csit.iterator_pertmf):
def __init__(self, lib_dir:str, h:str, lm_max_dlm:tuple,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few new parameters that lack docstrings

@Sebastian-Belkner
Copy link
Member

@Saladino93 what's the status here? looks like we have merge conflicts

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants