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

Cube mosaicing: Expand capabilities substantially #868

Draft
wants to merge 36 commits into
base: master
Choose a base branch
from

Conversation

keflavich
Copy link
Contributor

the cube utils mosaic header maker wasn't checking the spectral dimension before

@keflavich
Copy link
Contributor Author

Now branched off of #854

@keflavich keflavich changed the title Cube mosaicing: check spectral dimension Cube mosaicing: Expand capabilities substantially Mar 13, 2023
@keflavich keflavich mentioned this pull request Mar 13, 2023
@keflavich
Copy link
Contributor Author

This is largely reimplementing reproject/mosaicking/coadd.py, which is not good.

@keflavich
Copy link
Contributor Author

Some of the functionality I want here needs to be pushed into coadd. Will split this into another PR.

@keflavich
Copy link
Contributor Author

Dang, I thought I was getting somewhere.

Traceback (most recent call last):
  File "<ipython-input-1-349e0fbab013>", line 1, in <module>
    from aces.imaging.mosaic_12m import cs21_cube_mosaicing_minimal_test; cs21_cube_mosaicing_minimal_test()
  File "/orange/adamginsburg/ACES/reduction_ACES/aces/imaging/mosaic_12m.py", line 288, in cs21_cube_mosaicing_minimal_test
    result,_ = mosaic_cubes(cubes,
  File "/blue/adamginsburg/adamginsburg/repos/spectral-cube/spectral_cube/cube_utils.py", line 914, in mosaic_cubes
    output_array, output_footprint = reproject_and_coadd(
  File "/blue/adamginsburg/adamginsburg/repos/reproject/reproject/mosaicking/coadd.py", line 223, in reproject_and_coadd
    array, footprint = reproject_function(
  File "/blue/adamginsburg/adamginsburg/repos/astropy/astropy/utils/decorators.py", line 604, in wrapper
    return function(*args, **kwargs)
  File "/blue/adamginsburg/adamginsburg/repos/reproject/reproject/interpolation/high_level.py", line 121, in reproject_interp
    return _reproject_blocked(
  File "/blue/adamginsburg/adamginsburg/repos/reproject/reproject/utils.py", line 297, in _reproject_blocked
    output_array_dask = da.empty(shape_out, chunks=block_size or "auto")
  File "/blue/adamginsburg/adamginsburg/repos/dask/dask/array/wrap.py", line 60, in wrap_func_shape_as_first_arg
    parsed = _parse_wrap_args(func, args, kwargs, shape)
  File "/blue/adamginsburg/adamginsburg/repos/dask/dask/array/wrap.py", line 30, in _parse_wrap_args
    chunks = normalize_chunks(chunks, shape, dtype=dtype)
  File "/blue/adamginsburg/adamginsburg/repos/dask/dask/array/core.py", line 3106, in normalize_chunks
    raise ValueError(
ValueError: Chunks do not add up to shape. Got chunks=((4,), (5, 80, 101), (5, 100, 101)), shape=(4, 161, 152)

@keflavich
Copy link
Contributor Author

It works with a simple test - but not with blocks yet
image

@keflavich
Copy link
Contributor Author

I've been using this in production for a while. @e-koch, want to give it a once-over? Looks like I have some failing tests to fix, but I'm not sure if they're also failing on master

@codecov
Copy link

codecov bot commented Sep 3, 2023

Codecov Report

Patch has no changes to coverable lines.

❗ Current head e8083b7 differs from pull request most recent head 0cfcd02. Consider uploading reports for the commit 0cfcd02 to get more accurate results

Files Changed Coverage
spectral_cube/cube_utils.py 0.00%

📢 Thoughts on this report? Let us know!.

@keflavich
Copy link
Contributor Author

I added some tests in, and they revealed that something is going very weird with weighted mosaicking in at least one case (weights are going to nan when the data are fine).

cube-based reprojection (i.e., fully relying on reproject) is extrapolating in a region where it shouldn't, but that's literally an edge case, so I'm not so worried about it.

@keflavich
Copy link
Contributor Author

I'm now pretty confident there's a bug in reproject, or at least in how it's interacting with cube reprojection.

For one channel in the test case, I get:

weights: [[[nan nan nan nan nan nan]
  [nan nan nan nan nan nan]
  [nan nan nan nan nan nan]
  [nan nan nan nan nan nan]]]
weights_in: [[[1. 1. 1. 1. 1.]
  [1. 1. 1. 1. 1.]
  [1. 1. 1. 1. 1.]
  [1. 1. 1. 1. 1.]]

 [[1. 1. 1. 1. 1.]
  [1. 1. 1. 1. 1.]
  [1. 1. 1. 1. 1.]
  [1. 1. 1. 1. 1.]]],

while for the exact same input parameters (same WCS, same everything except it's "array" instead of "weights":

array: [[[2. 2. 2. 2. 2. 2.]
  [2. 2. 2. 2. 2. 2.]
  [2. 2. 2. 2. 2. 2.]
  [2. 2. 2. 2. 2. 2.]]]
array_in: [[[2. 2. 2. 2. 2.]
  [2. 2. 2. 2. 2.]
  [2. 2. 2. 2. 2.]
  [2. 2. 2. 2. 2.]]

 [[2. 2. 2. 2. 2.]
  [2. 2. 2. 2. 2.]
  [2. 2. 2. 2. 2.]
  [2. 2. 2. 2. 2.]]

 [[2. 2. 2. 2. 2.]
  [2. 2. 2. 2. 2.]
  [2. 2. 2. 2. 2.]
  [2. 2. 2. 2. 2.]]]

Why are weights going to NaN?

@keflavich
Copy link
Contributor Author

this raises a different issue, which is that weights are implicitly on the same grid as the data, but they're not actually required to be

@keflavich
Copy link
Contributor Author

I fixed the bad weights issue in reproject/5cfa233 / astropy/reproject#474. However, I still don't understand how I encountered the error in the first place - the weight & cube headers are identical.

@keflavich
Copy link
Contributor Author

Checking through errors... there are some horrendous ones. test_mosaic_cubes is resulting in super wrong values:

result.filled_data[:]
<Quantity [[[ 2.68308185e+185, -2.51563734e+252],
            [-9.03978759e+039,  3.04370599e+222],
            [-1.22330481e+051,  2.72896124e+167]],

           [[ 1.51095075e+196, -1.41665604e+263],
            [-5.09066610e+050,  1.71403263e+233],
            [-6.88891886e+061,  1.53678727e+178]],

           [[-2.40002193e+125,  8.57903219e+276],
            [-7.38140426e+135,  6.26917649e+173],
            [ 9.18684009e+002,  1.58752265e-008]],

           [[-1.09033779e+114,  3.89622506e+287],
            [-3.35339603e+124,  2.84810733e+162],
            [ 4.17361142e-009,  7.21216719e-020]]] K>

I think this indicates that an np.empty array is allocated and filled?

This could be associated with memmap; I can't tell yet (waiting on tests to finish).

@keflavich
Copy link
Contributor Author

following up - it's not exclusive to memmap or mode:

FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes[True-100-False0] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes[True-100-False1] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes[True-1-True] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes[True-1-False] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[False-1-True-channel] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[False-1-False-channel] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[False-100-True-channel] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[False-100-False-channel] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[False-None-True-channel] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[False-None-False-channel] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-1-True-cube] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-1-True-channel] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-1-False-cube] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-1-False-channel] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-100-True-cube] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-100-True-channel] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-100-False-cube] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-100-False-channel] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-None-True-channel] - AssertionError:
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-None-False-channel] - AssertionError:

@keflavich
Copy link
Contributor Author

Test failures are down to

FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-1-True-channel] - RuntimeError: IOLoop is closed
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-1-False-channel] - RuntimeError: IOLoop is closed
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-100-True-channel] - RuntimeError: IOLoop is closed
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-100-False-channel] - RuntimeError: IOLoop is closed
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-None-True-channel] - RuntimeError: IOLoop is closed
FAILED spectral_cube/tests/test_regrid.py::test_mosaic_cubes_spectral[True-None-False-channel] - RuntimeError: IOLoop is closed
FAILED spectral_cube/tests/test_regrid.py::test_cube_mosaic_weighted - TypeError: not all arguments converted during string formatting

@keflavich
Copy link
Contributor Author

those configurations are impossible to read.

test_mosaic_cubes_spectral[True-None-False-channel]

means

use_memmap = False, data_adv = PosixPath('/private/var/folders/k_/7qh4l0nn72b7qgq15pkd4hw40000gt/T/pytest-of-adam/pytest-146/test_mosaic_cubes_spectral_Tru11/adv.fits'), use_dask = True, spectral_block_size = None, method = 'channel'
test_mosaic_cubes_spectral[True-None-True-channel]

means

use_memmap = True, data_adv = PosixPath('/private/var/folders/k_/7qh4l0nn72b7qgq15pkd4hw40000gt/T/pytest-of-adam/pytest-146/test_mosaic_cubes_spectral_Tru9/adv.fits'), use_dask = True, spectral_block_size = None, method = 'channel'
test_mosaic_cubes_spectral[True-100-False-channel]

means

use_memmap = False, data_adv = PosixPath('/private/var/folders/k_/7qh4l0nn72b7qgq15pkd4hw40000gt/T/pytest-of-adam/pytest-146/test_mosaic_cubes_spectral_Tru7/adv.fits'), use_dask = True, spectral_block_size = 100, method = 'channel'

so the error is just all use_dask examples with channel mode.

@keflavich keflavich marked this pull request as draft January 26, 2025 17:37
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.

1 participant