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

Fix circular logic in Thin Lens deflection angle calculation for multiplane code #242

Open
AlexandreAdam opened this issue Jul 19, 2024 · 1 comment · May be fixed by #306
Open

Fix circular logic in Thin Lens deflection angle calculation for multiplane code #242

AlexandreAdam opened this issue Jul 19, 2024 · 1 comment · May be fixed by #306

Comments

@AlexandreAdam
Copy link
Collaborator

In the ThinLens class we have method

    def physical_deflection_angle(
        self,
        x: Tensor,
        y: Tensor,
        z_s: Tensor,
        *args,
        params: Optional["Packed"] = None,
        z_l: Optional[Tensor] = None,
        **kwargs,
    ) -> tuple[Tensor, Tensor]:
        """
        Computes the physical deflection angle immediately after passing through this lens's plane.

        Parameters
        ----------
        x: Tensor
            Tensor of x coordinates in the lens plane.

            *Unit: arcsec*

        y: Tensor
            Tensor of y coordinates in the lens plane.

            *Unit: arcsec*

        z_s: Tensor
            Tensor of source redshifts.

            *Unit: unitless*

        params: (Packed, optional)
            Dynamic parameter container for the lens model. Defaults to None.

        Returns
        -------
        x_component: Tensor
            Deflection Angle in x-direction.

            *Unit: arcsec*

        y_component: Tensor
            Deflection Angle in y-direction.

            *Unit: arcsec*

        """
        d_s = self.cosmology.angular_diameter_distance(z_s, params)
        d_ls = self.cosmology.angular_diameter_distance_z1z2(z_l, z_s, params)
        deflection_angle_x, deflection_angle_y = self.reduced_deflection_angle(
            x, y, z_s, params
        )
        return func.physical_from_reduced_deflection_angle(
            deflection_angle_x, deflection_angle_y, d_s, d_ls
        )

We also have method

    def reduced_deflection_angle(
        self,
        x: Tensor,
        y: Tensor,
        z_s: Tensor,
        *args,
        params: Optional["Packed"] = None,
        z_l: Optional[Tensor] = None,
        **kwargs,
    ) -> tuple[Tensor, Tensor]:
        """
        Computes the reduced deflection angle of the lens at given coordinates [arcsec].

        Parameters
        ----------
        x: Tensor
            Tensor of x coordinates in the lens plane.

            *Unit: arcsec*

        y: Tensor
            Tensor of y coordinates in the lens plane.

            *Unit: arcsec*

        z_s: Tensor
            Tensor of source redshifts.

            *Unit: unitless*

        params: (Packed, optional)
            Dynamic parameter container for the lens model. Defaults to None.

        Returns
        --------
        x_component: Tensor
            Deflection Angle in the x-direction.

            *Unit: arcsec*

        y_component: Tensor
            Deflection Angle in the y-direction.

            *Unit: arcsec*

        """
        d_s = self.cosmology.angular_diameter_distance(z_s, params)
        d_ls = self.cosmology.angular_diameter_distance_z1z2(z_l, z_s, params)
        deflection_angle_x, deflection_angle_y = self.physical_deflection_angle(
            x, y, z_s, params
        )
        return func.reduced_from_physical_deflection_angle(
            deflection_angle_x, deflection_angle_y, d_s, d_ls
        )

The implementation logic is circular. Furthermore, the physical deflection angle convention in the literature is to take in physical coordinates. We decided a while ago to use the convention of using angular coordinates in caustics, since our main target was the reduced deflection angle method.

I'd like to review this part of the code with the intention to refactor and clarify the multiplane code.

@ConnorStoneAstro
Copy link
Member

The intention with the circular logic was to make it so only one needed to be defined and the other would come automatically. If that seems ok to you, maybe we can close this issue?

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 a pull request may close this issue.

2 participants