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

Fix Factorial2 #308

Closed
Ali-Tehrani opened this issue Feb 8, 2024 · 4 comments · Fixed by #319
Closed

Fix Factorial2 #308

Ali-Tehrani opened this issue Feb 8, 2024 · 4 comments · Fixed by #319
Assignees

Comments

@Ali-Tehrani
Copy link
Contributor

Ali-Tehrani commented Feb 8, 2024

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 Ali-Tehrani self-assigned this Feb 8, 2024
@PaulWAyers
Copy link
Member

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

@Ali-Tehrani
Copy link
Contributor Author

Ali-Tehrani commented Feb 9, 2024

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

@D-TheProgrammer
Copy link
Contributor

D-TheProgrammer commented Feb 29, 2024

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

@Ali-Tehrani
Copy link
Contributor Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment