iodata icon indicating copy to clipboard operation
iodata copied to clipboard

Fix Factorial2

Open Ali-Tehrani opened this issue 2 years ago • 4 comments

The most recent merged pull-request causes 165 tests to fail in Github Actions for various python systems. The following error is obtained for the vast majority of them:

     def factorial2(n, exact=False):
        """Wrap scipy.special.factorial2 to return 1.0 when the input is -1.
    
        This is a temporary workaround while we wait for Scipy's update.
        To learn more, see https://github.com/scipy/scipy/issues/18409.
    
        Parameters
        ----------
        n : int or np.ndarray
            Values to calculate n!! for. If n={0, -1}, the return value is 1.
            For n < -1, the return value is 0.
        """
        # Scipy  1.11.x returns an integer when n is an integer, but 1.10.x returns an array,
        # so np.array(n) is passed to make sure the output is always an array.
        out = scipy.special.factorial2(np.array(n), exact=exact)
>       out[out <= 0] = 1.0
E       TypeError: 'int' object does not support item assignment

Further the following line does not match scipy.factorial2 API. Looking at the earlier release of this package, we want exact=True in scipy.factorial2. When setting exact=True, then scipy.factorial2 only accepts integers, and so we will need to run it over each individual element of the array.

The solution would be too in factorial2 function:

  1. Keep out = scipy.special.factorial2(np.array(n), exact=exact) when exact=False
  2. When exact=True, and i. Add if n is not int, then convert it to array n = np.array([n])) ii. compute scipy.special.factorial2(x, exact) on each individual element x of the array n.

Thank you to @LudoRNLT and @D-TheProgrammer, if you got the time to look into this.

Ali-Tehrani avatar Feb 08 '24 13:02 Ali-Tehrani

@Ali-Tehrani is this also going to be an issue in GBasis?

PaulWAyers avatar Feb 08 '24 22:02 PaulWAyers

I doubt it will be a problem at all, since GBasis will only go up to like 7-11!! and the performance advantages from vectorization are much greater

Ali-Tehrani avatar Feb 09 '24 15:02 Ali-Tehrani

Sorry to bother you after all this time. I've been really busy. I apologize for asking you this now, but could you please explain to me how I can use a file test from 'iodata/test' ? I've tried using the simple command 'python3 common.py', but it doesn't seem to work.

image

I initially thought it was a problem because the 'setup.py' was in the root directory of the project. So, I attempted this command instead: 'python3 -m iodata.test.common.py', but unfortunately, it didn't work either.

Once again I am sorry to asking you this @Ali-Tehrani

D-TheProgrammer avatar Feb 29 '24 10:02 D-TheProgrammer

No worries, if you want to run in the format you specified, you should be removing the .py and be in the iodata directory:

python3 -m iodata.test.common

However, I would recommend to use pytest instead to see that the tests pass like so:

pytest -v -s ./iodata/test/*.py   # -s means it prints while it runs

Adding -s makes it easier to debug and see what's being printed. Good luck, let me know if you have any other questions!

Ali-Tehrani avatar Feb 29 '24 14:02 Ali-Tehrani