pmelchior / scarlet

hyperspectral galaxy modeling and deblending
MIT License
49 stars 22 forks source link

expand_dims fix breaks .match(model_frame) #239

Closed patrickaleo closed 3 years ago

patrickaleo commented 3 years ago

Hi. I am following the docs tutorial here from the scarlet codebase: https://pmelchior.github.io/scarlet/0-quickstart.html

I noticed that in commit 216e7813be6b205cf8506b2e42646f6c4644c5e0 there was a change to fix a depreciation, where axis in np.expand_dims(model, axis=tuple(range(self._d))) is now a tuple, which I believe may throw a TypeError when running .match. Here is the cell block I am running:

model_frame = scarlet.Frame(
    images.shape,
    psf=model_psf,
    channels=filters)

observation = scarlet.Observation(
    images, 
    psf=psf,
    weights=weights, 
    channels=filters).match(model_frame)

and here is the full traceback:

type_error

(I have run all cells perfectly before that, exactly matching the quick start tutorial.) Any advice would be greatly appreciated. Thanks!

patrickaleo commented 3 years ago

update: I still haven't figured out a solution modifying axis, so your thoughts are greatly appreciated!

pmelchior commented 3 years ago

I can't reproduce this problem. I run the code on python 3.7 without failure. I also tested autograd's numpy implementation for a range of dimensions. Can you check what your PSF instance reports as the ._d attribute. This should be an integer, normally 1.

patrickaleo commented 3 years ago

I appreciate your help. I checked and my model_psf._d = 1. Though I also noticed that the line in the second cell of the tutorial psf = scarlet.ImagePSF(data["psfs"]), that this psf instance has many attributes but no _d attribute. It has ['abstractmethods', 'class', 'delattr', 'dict', 'dir', 'doc', 'eq', 'format', 'ge', 'getattribute', 'getitem', 'gt', 'hash', 'init', 'init_subclass', 'iter', 'le', 'lt', 'module', 'ne', 'new', 'next', 'reduce', 'reduce_ex', 'repr', 'setattr', 'sizeof', 'slots', 'str', 'subclasshook', 'weakref', '_abc_impl', '_children', '_parameters', 'bbox', 'check_parameters', 'children', 'get_model', 'get_models_of_children', 'get_parameter', 'parameters', 'prepare_param', 'update']

Not sure if that's helpful. (And yes I'm also running Python 3.7)

patrickaleo commented 3 years ago

my scarlet version is '1.0.1+gfde109a' by the way, and I'm following the tutorial exactly as written

fred3m commented 3 years ago

Hi Patrick. We have tested this on multiple machines (mc OS, centos 7, centos 8), including in the LSST tests that run multiple times daily. The docs themselves are built with each PR on a fresh environment and are still building and running correctly.

So the most likely source of this is something in your python environment, most likely due to an old version of numpy. Could you please check (and potentially upgrade) your numpy version. If that doesn't work can you create a new clean environment and a fresh install of scarlet? We don't currently check the numpy version in the scarlet install but if this does fix your issue please let us know so that we can implement a minimum version.

patrickaleo commented 3 years ago

I really appreciate the help, and upgrading numpy worked (sorry I didn't think of this earlier)! Now the tutorial runs perfectly. Originally I had numpy=1.17.4 (a clone of the default conda environment for our computing cluster), and now I have upgraded tonumpy=1.20.1 which works beautifully. So yes, I think it is wise to include a minimum version of numpy now in your scarlet install. Cheers!

fred3m commented 3 years ago

Great, thanks for the feedback. @pmelchior can you update this as part of #234 ? I think that a minimum numpy version of 1.18.0 should work, but it might require 1.19.0.