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

[BUG] zoom_center_x/y incorrectly calculates center when aligned by WCS #3225

Open
duytnguyendtn opened this issue Oct 17, 2024 · 9 comments
Labels
bug Something isn't working needs-triage Issue opened via template and needs triaging

Comments

@duytnguyendtn
Copy link
Collaborator

duytnguyendtn commented Oct 17, 2024

Jdaviz component

Imviz

Description

Hi again!

This bug is a companion to #3217, as I thought the initialization was the source of the two errors that cropped up in #2872, but turns out #3222 only fixed one of them. I dug into the other failure and found this:

In one of the test examples for #2872, the zoom_center_x/y produces the correct result when pixel linked, but once switching to WCS alignment, trips off an assertion error. It looks like the second WCS coordinate abs value is correct, just that it isn't negated. The marker is where I eye-balled the center of the viewer to actually be:

image

How to Reproduce

Should be done in a jupyter environment to show the viewer graphically

from jdaviz import Imviz
from astropy.io import fits
from astropy.coordinates import SkyCoord
import numpy as np

imviz_helper = Imviz()

# First data with WCS, same as the one in BaseImviz_WCS_NoWCS.
hdu1 = fits.ImageHDU(np.random.rand(100, 100), name="SCI")
hdu1.header.update(
    {
        "CTYPE1": "RA---TAN",
        "CUNIT1": "deg",
        "CDELT1": -2.777777778,
        "CRPIX1": 1,
        "CRVAL1": 337.5202808,
        "NAXIS1": 10,
        "CTYPE2": "DEC--TAN",
        "CUNIT2": "deg",
        "CDELT2": 2.777777778,
        "CRPIX2": 1,
        "CRVAL2": -20.833333059999998,
        "NAXIS2": 10,
    }
)
imviz_helper.load_data(hdu1, data_label="has_wcs_1")

imviz_helper.plugins["Orientation"].align_by = "WCS"

imviz_helper.show()

Wait for the viewer to "settle" then execute in another cell:

state = imviz_helper.default_viewer._obj.state
print(f"WCS Coords: {state.zoom_center_x}, {state.zoom_center_y}")
print(f"Pixel Coords: {state.reference_data.coords.world_to_pixel(SkyCoord(state.zoom_center_x, state.zoom_center_y, unit='deg'))}")

Returned result (In my viewer and screenshot):

WCS Coords: 327.43033887933575, 9.113814207172757
Pixel Coords: [2.6032673415361294, 4.9523513137498]

Expected behavior

Eyeballed result should be closer to (3.8, 4.1) pixel coord or (327.37577, -9.18857) WCS coord, I think. I think the second coordinate is not being negated properly

Browser

Chrome 129.0.6668.100

Jupyter

See below

Software versions

anyio==4.6.0
argon2-cffi==23.1.0
argon2-cffi-bindings==21.2.0
arrow==1.3.0
asdf==3.5.0
asdf-astropy==0.6.1
asdf_coordinates_schemas==0.3.0
asdf_standard==1.1.1
asdf_transform_schemas==0.5.0
asdf_wcs_schemas==0.4.0
asteval==1.0.5
astropy==6.1.4
astropy-iers-data==0.2024.10.7.0.32.46
astroquery==0.4.7
asttokens==2.4.1
async-lru==2.0.4
attrs==24.2.0
babel==2.16.0
backports.tarfile==1.2.0
beautifulsoup4==4.12.3
bleach==6.1.0
bqplot==0.12.43
bqplot-gl==0.0.0
bqplot-image-gl==1.4.11
cachetools==5.5.0
casa-formats-io==0.3.0
certifi==2024.8.30
cffi==1.17.1
charset-normalizer==3.4.0
click==8.1.7
cloudpickle==3.0.0
colorama==0.4.6
comm==0.2.2
contourpy==1.3.0
coverage==7.6.2
cycler==0.12.1
dask==2024.9.1
debugpy==1.8.6
decorator==5.1.1
defusedxml==0.7.1
dill==0.3.9
echo==0.9.0
et-xmlfile==1.1.0
executing==2.1.0
fast-histogram==0.14
fastjsonschema==2.20.0
filelock==3.16.1
fonttools==4.54.1
fqdn==1.5.1
freetype-py==2.5.1
fsspec==2024.9.0
glfw==2.7.0
glue-astronomy==0.10.0
glue-core==1.21.1
glue-jupyter==0.22.2
glue-vispy-viewers==1.2.2
gwcs==0.21.0
h11==0.14.0
hsluv==5.0.4
html5lib==1.1
httpcore==1.0.6
httpx==0.27.2
humanize==4.11.0
hypothesis==6.114.1
idna==3.10
imageio==2.35.1
importlib_metadata==8.5.0
iniconfig==2.0.0
ipydatawidgets==4.3.5
ipygoldenlayout==0.4.0
ipykernel==6.29.5
ipympl==0.9.4
ipypopout==1.4.0
ipysplitpanes==0.2.0
ipython==8.28.0
ipython-genutils==0.2.0
ipyvolume==0.6.3
ipyvue==1.11.1
ipyvuetify==1.10.0
ipywebrtc==0.6.0
ipywidgets==8.1.5
isoduration==20.11.0
jaraco.classes==3.4.0
jaraco.context==6.0.1
jaraco.functools==4.1.0
-e git+https://github.com/duytnguyendtn/jdaviz.git@e40e62498123c964c21b51587f76c5c7e27aa090#egg=jdaviz
jedi==0.19.1
Jinja2==3.1.4
jmespath==1.0.1
joblib==1.4.2
json5==0.9.25
jsonpointer==3.0.0
jsonschema==4.23.0
jsonschema-specifications==2024.10.1
jupyter-events==0.10.0
jupyter-lsp==2.2.5
jupyter-rfb==0.4.4
jupyter_client==8.6.3
jupyter_core==5.7.2
jupyter_server==2.14.2
jupyter_server_terminals==0.5.3
jupyterlab==4.2.5
jupyterlab_pygments==0.3.0
jupyterlab_server==2.27.3
jupyterlab_widgets==3.0.13
keyring==25.4.1
kiwisolver==1.4.7
lazy_loader==0.4
locket==1.0.0
Markdown==3.7
markdown-it-py==3.0.0
MarkupSafe==3.0.1
matplotlib==3.9.2
matplotlib-inline==0.1.7
mdurl==0.1.2
mistune==3.0.2
more-itertools==10.5.0
mpl-scatter-density==0.7
nbclient==0.10.0
nbconvert==7.16.4
nbformat==5.10.4
ndcube==2.2.2
nest-asyncio==1.6.0
networkx==3.3
notebook==7.2.2
notebook_shim==0.2.4
numpy==2.1.2
openpyxl==3.1.5
overrides==7.7.0
packaging==24.1
pandas==2.2.3
pandocfilters==1.5.1
parso==0.8.4
partd==1.4.2
photutils==1.13.0
pillow==10.4.0
platformdirs==4.3.6
pluggy==1.5.0
prometheus_client==0.21.0
prompt_toolkit==3.0.48
psutil==6.0.0
pure_eval==0.2.3
pycparser==2.22
pyerfa==2.0.1.4
Pygments==2.18.0
pymdown-extensions==10.11.2
PyOpenGL==3.1.7
pyparsing==3.1.4
pytest==8.3.3
pytest-arraydiff==0.6.1
pytest-astropy==0.11.0
pytest-astropy-header==0.2.2
pytest-cov==5.0.0
pytest-doctestplus==1.2.1
pytest-filter-subpackage==0.2.0
pytest-mock==3.14.0
pytest-remotedata==0.4.1
pytest-tornasync==0.6.0.post2
python-dateutil==2.9.0.post0
python-json-logger==2.0.7
pythreejs==2.4.2
pytz==2024.2
pyvo @ git+https://github.com/astropy/pyvo.git@e7017544ef76d831236bbb7ee67d62aedb967dcc
pywin32==307
pywin32-ctypes==0.2.3
pywinpty==2.0.13
PyYAML==6.0.2
pyzmq==26.2.0
radio-beam==0.3.7
reacton==1.8.3
referencing==0.35.1
regions==0.10
requests==2.32.3
rfc3339-validator==0.1.4
rfc3986-validator==0.1.1
rich==13.9.2
rich-click==1.8.3
rpds-py==0.20.0
scikit-image==0.24.0
scipy==1.14.1
semantic-version==2.10.0
Send2Trash==1.8.3
shapely==2.0.6
sidecar==0.7.0
six==1.16.0
sniffio==1.3.1
solara==1.39.0
solara-server==1.39.0
solara-ui==1.39.0
sortedcontainers==2.4.0
soupsieve==2.6
specreduce==1.4.1
spectral-cube==0.6.5
specutils==1.17.0
stack-data==0.6.3
starlette==0.39.2
stdatamodels==2.1.1
terminado==0.18.1
tifffile==2024.9.20
tinycss2==1.3.0
toolz==1.0.0
tornado==6.4.1
traitlets==5.14.3
traittypes==0.2.1
types-python-dateutil==2.9.0.20241003
typing_extensions==4.12.2
tzdata==2024.2
uri-template==1.3.0
urllib3==2.2.3
uvicorn==0.31.1
vispy==0.14.3
watchdog==5.0.3
watchfiles==0.24.0
wcwidth==0.2.13
webcolors==24.8.0
webencodings==0.5.1
websocket-client==1.8.0
websockets==13.1
widgetsnbextension==4.0.13
xlrd==2.0.1
zipp==3.20.2

@duytnguyendtn duytnguyendtn added bug Something isn't working needs-triage Issue opened via template and needs triaging labels Oct 17, 2024
@duytnguyendtn
Copy link
Collaborator Author

Specifically for #2872 I just need to know what truth value to put into the unit test. The test example here is just a copy of one of the existing imviz test fixtures. But this does open up a question of what the plugin should trust... I'd be concerned someone attempts to use a highly skewed WCS file and getting incorrect coordinates

@duytnguyendtn duytnguyendtn changed the title [BUG] zoom_center_x/y incorrectly calculates WCS center [BUG] zoom_center_x/y incorrectly calculates center when aligned by WCS Oct 17, 2024
@kecnry
Copy link
Member

kecnry commented Oct 17, 2024

I see this is only with one image layer, but is the app in WCS or pixel linking mode?

See https://github.com/spacetelescope/jdaviz/blob/bef7a001f1a61a0df034c730c08d2843b1d82227/jdaviz/core/freezable_state.py#L131-145 for the relevant logic being triggered here:

        # When WCS-linked (displayed on the sky): zoom_center_x/y and zoom_radius are in sky units,
        # x/y_min/max are in pixels of the WCS-only layer
        if self.linked_by_wcs:
            image, i_ref = get_reference_image_data(self._viewer.jdaviz_app, self._viewer.reference)
            ref_wcs = image.coords
            lims = ref_wcs.pixel_to_world_values((self.x_min, self.x_max), (self.y_min, self.y_max))
            x_min, x_max = lims[0]
            y_min, y_max = lims[1]
        else:
            x_min, y_min = self.x_min, self.y_min
            x_max, y_max = self.x_max, self.y_max
        # now x_min/max, y_min/max are in axes units (degrees if WCS-linked, pixels otherwise)

        with self.during_zoom_sync():
            self.zoom_radius = abs(0.5 * min(x_max - x_min, y_max - y_min))
            self.zoom_center_x = 0.5 * (x_max + x_min)
            self.zoom_center_y = 0.5 * (y_max + y_min)

@duytnguyendtn
Copy link
Collaborator Author

The app is in WCS alignment mode; I realized the title was ambiguous and modified the title for clarity.

But yes, I was able to reproduce this with only one data layer. The tool is aligning it against the orientation layer, right?

@kecnry
Copy link
Member

kecnry commented Oct 17, 2024

ok, in that case I suspect the bug is somewhere in that if-statement or how the orientation WCS handles mapping this flat onto the sky (cc @bmorris3)

@duytnguyendtn
Copy link
Collaborator Author

Just to check my assumptions here: zoom_center_x/y is intended to report the center PIXEL coordinates, correct?

@kecnry
Copy link
Member

kecnry commented Oct 17, 2024

no, in this case they will be in world coordinates (and are exposed as editable at the top of plot options, with units attached). In that case, maybe this is correct? I can't tell where your mouse is in the screenshot, are the world coordinates of the marker matching the values provided?

@duytnguyendtn
Copy link
Collaborator Author

The mouse in the screenshot is centered on the marker in the image.

Oooh, well maybe this is where my confusion is coming from. (I mean, we still have the issue the zoom_center_x/y doesn't match the WCS coords either...). Going back to #3217, the fix for it resulted in that example's zoom_center_x/y to have values of 49.5, 49.5, which I thought were pixel coordinates (in that bug, the alignment was still in pixels). 49.5,49.5 makes as center PIXEL coordinates, considering the data array is of size 100,100. Does this mean that zoom_center_x/y reports its coordinates based on how it's aligned, and switches when the alignment type changes?

@kecnry
Copy link
Member

kecnry commented Oct 17, 2024

Does this mean that zoom_center_x/y reports its coordinates based on how it's aligned, and switches when the alignment type changes?

Yes

Screen.Recording.2024-10-17.at.12.05.28.PM.mov

@duytnguyendtn
Copy link
Collaborator Author

duytnguyendtn commented Oct 30, 2024

I took another look at this and have a couple findings:

  1. The center coordinates shift slightly when the viewer is shown, at least when WCS linked. This shift seems to be enough to trigger the assert_all_close check to fail, resulting in the discrepancy of the failing test case in Virtual Observatory plugin for Jdaviz #2872. Knowing this, I'll adjust the new truth values in the test to match the new values when the viewer is not being shown.
  2. Knowing that, when I recalculate the cell that prints the coordinates after the viewer "settles," the coordinate values actually seem correct, just that one of them is positive, when the mouseover shows it should be negative. I've adjusted the bug description to match accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-triage Issue opened via template and needs triaging
Projects
None yet
Development

No branches or pull requests

2 participants