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

Interpolation light profile and inverted coordinates #176

Open
aymgal opened this issue Apr 13, 2020 · 6 comments
Open

Interpolation light profile and inverted coordinates #176

aymgal opened this issue Apr 13, 2020 · 6 comments

Comments

@aymgal
Copy link
Collaborator

aymgal commented Apr 13, 2020

The 'INTERPOL' light profile does not accept coordinates arrays, like it is the case of the 'INTERPOL' mass profile. I think it instead considers that coordinates are positive going right for x and up for y.

This leads to the interpolated surface brightness map being incorrectly oriented when the coordinates were obtained through inverse=True (in utility functions), i.e. East is on the left instead of right.

@sibirrer
Copy link
Collaborator

The design in the light profile 'INTERPOL' is slightly different but it should accept coordinate arrays. They just need to be one-dimensional. The light profile features a rotation component, so you can rotate your 2d map in different directions. The chosen coordinate system (inverse or not) should not affect the calculation of this class. In that sense it is 'just' that the input only supports rotations along coordinate and not transposes.
There is probably anyways a definition/class to be written for SLIT to map the coordinate system of a reconstruction (can also be rotated) to feed in the right angle into the INTERPOL light class (and a possible transpose). The question is whether you want to have this managed by the INTERPOL class or outside of it.

@aymgal
Copy link
Collaborator Author

aymgal commented Apr 14, 2020

Thanks for the answer. I can indeed find a way to initialise the interpol profile with the right orientation, but it would require to have access to the inverse property of the coordinates beforehand in order to detect the current orientation, which is not possible right now, outside of the Coordinates classes of lenstronomy.
What I do currently is that I check the sign of the first entry in the pixel-to-angle matrix. But this is not good. What would be your preference to check the orientation of the coordinates externally?
I could add a property to the Coordinates e.g.:

@property
def ra_inversed(self):
    return True if (self._Mpix2a[0, 0] < 0) else False 

@sibirrer
Copy link
Collaborator

Your proposal does not generally work if the coordinate system is rotated. I think the eigenvalues and eigenvectors of the mapping matrix can help you here.

@aymgal
Copy link
Collaborator Author

aymgal commented Apr 14, 2020

Right. Let's stick with an implementation of the interpol light profile, similarly with to the equivalent mass profile. I can propose a PR.
Before starting to implement it, do you prefer a new interpolation profile, or that I add the two extra parameters grid_interp_x, grid_interp_y to the existing function ?

Another possibility: I don't use the current implementation of the interpolation profile in the Starlets profile, and re-implement the interpolation functions inside the Starlets class with the specific features that I want. Is it your preference ? No need for any change in your interface in that case.

@sibirrer
Copy link
Collaborator

The grid_interp_x and grid_interp_y in the lens model module is also not optimal as it requires rectangular orientation. I'd prefer not having a mix of interpolation schemes defining a coordinate system with schemes defining an axis. The coordinate scheme is much more general and I hopefully can replace the lens model part with such a scheme too. So if you implement an new profile, I would prefer it to be one with inputs consistent with the PixelGrid(). It's a bit more work but can ensure consistency. Then the difference to what I coded up would be:

  • need to calculate the rotation angle and potential flipping based on the transformation matrix. Then you don't even have to worry about the axes themselves as the user from outside the class. Does this make sense?

@aymgal
Copy link
Collaborator Author

aymgal commented Apr 15, 2020

I agree this would make sense to support same arguments that coordinates-related classes. I'm not sure to come to a full implementation very soon, but I put it on my TODO list 👍

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

No branches or pull requests

2 participants