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

New EPL+powerlawmultipole profile (EPL+PLMultipole) #683

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

eckerl
Copy link

@eckerl eckerl commented Dec 6, 2024

A new combined profile of EPL+powerlawmultipole.

The PLMultipole is an extention to the parametrization of Chu et al.(2013) (https://arxiv.org/abs/1302.5482). The equation used is Eq. (8) from Nightingale et al. (2023) (https://arxiv.org/abs/2209.10566)
It scales the contribution of the multipoles with the same power-law as the EPL. It is defined with a prefactor 1/2 and the einstein radius theta_e in order to achieve k = theta_E / 2theta as the m=0 contribution (Reason stated in Chu et al.(2013) (https://arxiv.org/abs/1302.5482) Eq. (3)) The Multipole definition before is only valid in the isothermal case and should only be used with isothermal profiles. This is a profile which is already the combination of an EPL+PLMultipol and not its own multipole profile.

@sibirrer
Copy link
Collaborator

sibirrer commented Dec 6, 2024

Thank you very much @eckerl for the PR! @dangilman recently implemented some multipol models in a similar spirit in #682. @dangilman I add you as a reviewer here. I am sure there are things done slightly differently but to avoid too many duplications

@sibirrer sibirrer requested a review from dangilman December 6, 2024 14:35
@sibirrer
Copy link
Collaborator

sibirrer commented Dec 6, 2024

@eckerl there seems to be an import statement that is not done fully correctly that make the tests crash

Copy link
Collaborator

@dangilman dangilman left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution @eckerl! I left a couple of comments in the new profile class, mainly regarding missing docstrings.

Before merging, can you please add a test in test_numeric_lens_differentials for this profile? Also, I suggest you add this profile to the list of compatible lens models in the Solver4Point class


class EPL_PMultipol(LensProfileBase):
"""This class contains a EPL+PLMultipole contribution over e1,e2.
The PLMultipole is an extention to the parametrization of Chu et al.(2013) (https://arxiv.org/abs/1302.5482). The equation is Eq. (8) from Nightingale et al. (2023) (https://arxiv.org/abs/2209.10566)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please add note here that this is a circular multipole, rather than an elliptical multipole i.e. it introduces a multipole deviation around a circle rather than an ellipse.

Here, \( k^{\rm mass}_m \) and \( \phi^{\rm mass}_m \) are parameterized as elliptical components:
\[
\left(\epsilon_{\rm 1}^{\rm mp}, \epsilon_{\rm 2}^{\rm mp}\right)
\]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add note here that notation k_m will be used to denote a multipole strength for a non-isothermal profile, whereas a_m will be used for multipole perturbations to isothermal profiles

:param gamma: power law slope
:param e1: eccentricity component
:param e2: eccentricity component
:param center_x: profile center
Copy link
Collaborator

Choose a reason for hiding this comment

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

add docstrings for m and k_m

:param gamma: power law slope
:param e1: eccentricity component
:param e2: eccentricity component
:param center_x: profile center
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing docstrings for m and k_m

:param gamma: power law slope
:param q: axis ratio
:param phi: position angle
:param center_x: profile center
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing docstrings for m and k_m

:param theta_E: Einstein radius
:param gamma: power law slope
:param q: axis ratio
:param phi: position angle
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing docstrings for m and k_m

:param theta_E: Einstein radius
:param gamma: power law slope
:param q: axis ratio
:param phi: position angle
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing docstrings for m and k_m

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.

3 participants