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

Target class mask datatype in targets.py #79

Closed
apcooper opened this issue Sep 27, 2016 · 3 comments · Fixed by desihub/desiutil#45
Closed

Target class mask datatype in targets.py #79

apcooper opened this issue Sep 27, 2016 · 3 comments · Fixed by desihub/desiutil#45
Assignees

Comments

@apcooper
Copy link
Contributor

In processing the mws_galaxia mocks, the call to mask.names() here in desitarget.targets raises the following exception:

    239             bitnum = 0
    240             while bitnum**2 <= mask:
--> 241                 if (2**bitnum & mask):
    242                     if bitnum in self._bits.keys():
    243                         names.append(self._bits[bitnum].name)

TypeError: ufunc 'bitwise_and' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''

seemingly because the values that are being passed as tc are scalar np.(u)int64s. This only occurs for larger bitmask values, in this specific case when tc == 65536 (the 'very faint' selection in the mws_galaxia mocks): The relevant lines are:

    target_class_names = np.zeros(len(target_class),dtype=np.object)
    unique_target_classes = np.unique(target_class)
    for tc in unique_target_classes:
        # tc is the encoded integer value of the target bitmask
        has_this_target_class = np.where(target_class == tc)[0]

        tc_name = '+'.join(mask.names(tc))

Can be fixed by tc_name = '+'.join(mask.names(int(tc))) instead but doesn't seem that intuitive and I didn't see this flagged elsewhere -- should it be fixed in desiutil instead?

Some tests:

from desiutil.bitmask import BitMask
import yaml

_bitdefyaml = """\
ccdmask:
  - [BAD,              0, "Pre-determined bad pixel (any reason)"]
  - [HOT,              1, "Hot pixel", {'blat': 'foo'}]
  - [TEST,            16, "A higher bit for this test"]"""

_bitdefs = yaml.load(_bitdefyaml)
mask = BitMask('ccdmask' ,_bitdefs)

# Ok passing a python int directly
mask.names(65536)

# OK passing an unsigned array
a = np.array([65536],dtype=np.uint64)
mask.names(a)

# Fails if the array is signed
a = np.array([65536],dtype=np.int64)
mask.names(a)

# Fails with a numpy scalar, signed or unsigned
mask.names(np.int(65536))
mask.names(np.uint64(65536))

Numpy 1.11.1.

Possibly related:
numpy/numpy#2955
http://thread.gmane.org/gmane.comp.python.numeric.general/49242/focus=49338
http://stackoverflow.com/questions/26755352/numpy-bitwise-operation-error

@weaverba137
Copy link
Member

@sbailey should comment. I believe that an additional wrinkle in the problem is that the individual bits in desiutil.bitmask are subclasses of pure-Python int.

@sbailey sbailey self-assigned this Sep 27, 2016
@sbailey
Copy link
Contributor

sbailey commented Sep 27, 2016

Thanks for the bug report and examples; that makes this much easier to track down. It does seem to be a manifestation of numpy/numpy#2955 . A few others notes:

  • mask.names(np.int(65536)) works for me, though your report indicates that it fails for you.
  • the np.array([65536],dtype=np.int64) example fails for inputs as small as 2**12; though works for numbers smaller than that.
  • mask.names() was supposed to work with integer input (signed or unsigned, numpy or vanilla), but it wasn't intended to be used with arrays of inputs. I'm inclined to either throw a TypeError or return a list of lists, where the outer list as the same length as the input array. i.e. if mask.names(np.array([65536],dtype=np.uint64)) works, then mask.names(np.array([65536,65536],dtype=np.uint64) should also work and do something consistent.

Do you rely upon being able to pass arrays into mask.names()? I can see the usefulness of that, but that is a somewhat separate issue from how np.uint64 are treated.

@sbailey
Copy link
Contributor

sbailey commented Sep 27, 2016

The fix is in PR desihub/desiutil#45 . I left the array behavior as default: it works with a length(1) array but not other dimensions or lengths of arrays.

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.

3 participants