Promoters are matching too broadly

[seberg]

The promoters are matching PyArrayDescr_Type which is far too broadly, since they don't actually know how to do the right thing for NumPy inputs.

This breaks normal NumPy functioning. I get that it could be nice to just do a broad promotion rather than fine grained, but I don't understand that there is any reason for this here: They can just use Quaddtype (the only problem with that is that at times you have to add a combination of all promoters).

This issue means that the NumPy test-suite cannot pass when quaddtype is imported.

CC @SwayamInSync do you remember any reason for this?!

EDIT: And yes, I realize you need to register the promoter twice (and maybe more often, but I don't think you have that). And yes, it would be nice to modify things to allow this more conveniently, but we do need the precise version -- the convenience version may have to sacrifice having a defined order/priority.
It would likely be good to figure that out (i.e. ditching strict order by trusting promoters to do nothing or to do the right thing -- in which case we would have to continue with other promoters).

[ngoldbaum]

@seberg please correct me if I'm wrong, but I think it's ok to use PyArrayDescr_Type, but only if one of the DTypes is QuadPrecDType. So "doing it twice" here means something like:

      PyObject *DTypes_l = NULL, *DType_r = NULL, *promoter_capsule = NULL;

      DTypes_l = PyTuple_Pack(3, &QuadPrecDType, &PyArrayDescr_Type, &QuadPrecDType);

      if (DTypes_l; == 0) {
          goto cleanup;
      }

      PyObject *DTypes_r = PyTuple_Pack(3, &PyArrayDescr_Type, &QuadPrecDType, &QuadPrecDType);

      if (DTypes_r; == 0) {
          goto cleanup;
      }

      promoter_capsule =
              PyCapsule_New((void *)&quad_ufunc_promoter, "numpy._ufunc_promoter", NULL);

      if (promoter_capsule == NULL) {
          goto cleanup;
      }

      if (PyUFunc_AddPromoter(ufunc, DTypes_l, promoter_capsule) < 0) {
          goto cleanup;
      }

      if (PyUFunc_AddPromoter(ufunc, DTypes_r, promoter_capsule) < 0) {
          goto cleanup;
      }

cleanup:
      Py_XDECREF(DTypes_l);
      Py_XDECREF(DTypes_r);
      Py_XDECREF(promoter_capsule);

[seberg]

Yes maybe I was not clear. one input must be quaddtyppe to be sure you cannot override other promotion

[SwayamInSync]

@seberg yes I just checked and there are 3 cases (all are one of my earliest written codes) the promoter is registered with PyArrayDescr_Type which might be corrupting the global state.
I will drop a fix PR asap. Thanks for catching this!!

[SwayamInSync]

Also sorry for being out of the loop, ongoing research work at MSR has been taking up most of my time lately.
I expect things to ease up by the end of this month, and I’ll get back to clearing pending PRs/issues soon after

[ngoldbaum]

I took a stab at fixing this but didn't quite manage to get it working and ran out of steam today: https://github.com/numpy/numpy-quaddtype/compare/main...ngoldbaum:numpy-quaddtype:fix-promoters?expand=1

It's segfaulting in NumPy promoter internals, I'd need to trace a little to figure out what's going wrong.

[SwayamInSync]

Hey @ngoldbaum I pushed a PR SwayamInSync#7 on my fork, added the test and it seems to working.
I took the tests by running causing PR into my fork with removed guards, following were the original issues (visible in datetime, general ops seems to be working)

Click to expand

________ TestDateTime.test_timedelta_modulus_type_resolution[val10-15] _________
[gw0] darwin -- Python 3.12.10 /Library/Frameworks/Python.framework/Versions/3.12/bin/python

self = <test_datetime.TestDateTime object at 0x103b2d2e0>
val1 = np.timedelta64(7,'Y'), val2 = 15

    @pytest.mark.parametrize("val1, val2", [
        # cases where one operand is not
        # timedelta64
        (np.timedelta64(7, 'Y'),
         15,),
        (7.5,
         np.timedelta64(1, 'D')),
        ])
    def test_timedelta_modulus_type_resolution(self, val1, val2):
        # NOTE: some of the operations may be supported
        # in the future
>       with assert_raises_regex(TypeError,
                                 "'remainder' cannot use operands with types"):

self       = <test_datetime.TestDateTime object at 0x103b2d2e0>
val1       = np.timedelta64(7,'Y')
val2       = 15

numpy/_core/tests/test_datetime.py:2102: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py:263: in __exit__
    self._raiseFailure("{} not raised".format(exc_name))
        exc_name   = 'TypeError'
        exc_type   = None
        exc_value  = None
        self       = <unittest.case._AssertRaisesContext object at 0x10ef18830>
        tb         = None
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <unittest.case._AssertRaisesContext object at 0x10ef18830>
standardMsg = 'TypeError not raised'

    def _raiseFailure(self, standardMsg):
        msg = self.test_case._formatMessage(self.msg, standardMsg)
>       raise self.test_case.failureException(msg)
E       AssertionError: TypeError not raised

msg        = 'TypeError not raised'
self       = <unittest.case._AssertRaisesContext object at 0x10ef18830>
standardMsg = 'TypeError not raised'

/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py:200: AssertionError
___________________ TestDateTime.test_timedelta_correct_mean ___________________
[gw0] darwin -- Python 3.12.10 /Library/Frameworks/Python.framework/Versions/3.12/bin/python

self = <test_datetime.TestDateTime object at 0x103b7c230>

    def test_timedelta_correct_mean(self):
        # test mainly because it worked only via a bug in that allowed:
        # `timedelta.sum(dtype="f8")` to ignore the dtype request.
        a = np.arange(1000, dtype="m8[s]")
>       assert_array_equal(a.mean(), a.sum() / len(a))
E       AssertionError: 
E       Arrays are not equal
E       
E       Mismatched elements: 1 / 1 (100%)
E        ACTUAL: array(499.5)
E        DESIRED: array(499, dtype='timedelta64[s]')

a          = array([  0,   1,   2,   3,   4,   5,   6,   7,   8,   9,  10,  11,  12,
        13,  14,  15,  16,  17,  18,  19,  20,..., 984, 985, 986, 987,
       988, 989, 990, 991, 992, 993, 994, 995, 996, 997, 998, 999],
      dtype='timedelta64[s]')
self       = <test_datetime.TestDateTime object at 0x103b7c230>

numpy/_core/tests/test_datetime.py:2126: AssertionError

[SwayamInSync]

once all test pass will push to main and maybe if possible can we confirm inside the numpy/numpy#30798 by building from the source branch (just temporarily)?

[SwayamInSync]

I think I can do that check in my fork as well, nevermind!!

[SwayamInSync]

Seems to be working now here are the runner logs
https://github.com/SwayamInSync/numpy/actions/runs/23672096415/job/68967567214?pr=4

Here's the main PR #78