pytorch-forecasting icon indicating copy to clipboard operation
pytorch-forecasting copied to clipboard

Fix np.float and np.float32 to float.

Open BrunoBelucci opened this issue 2 years ago • 4 comments

Description

This PR fixes #1236 and #1207.

Following the numpy error message, np.float was a deprecated alias for the builtin float. The aliases was originally deprecated in NumPy 1.20.

Checklist

  • [x] Linked issues (if existing)
  • [ ] Amended changelog for large changes (and added myself there as contributor)
  • [ ] Added/modified tests
  • [ ] Used pre-commit hooks when committing to ensure that code is compliant with hooks. Install hooks with pre-commit install. To run hooks independent of commit, execute pre-commit run --all-files

Make sure to have fun coding!

BrunoBelucci avatar Feb 12 '23 17:02 BrunoBelucci

np.float is indeed depreacated, but I'm not sure np.float32 is also deprecated. I am pretty sure float is np.float64. So you should keep np.float32 expressions since GPU usually works with 32 bits floatting point numbers.

graille avatar Feb 15 '23 11:02 graille

np.float is indeed depreacated, but I'm not sure np.float32 is also deprecated. I am pretty sure float is np.float64. So you should keep np.float32 expressions since GPU usually works with 32 bits floatting point numbers.

You are absolutely right, I have assumed the inverse, that float was np.float32. Therefore, I should not have changed np.float32 to float, but it is not clear to me when we are doing eps = np.finfo(float).eps if we want 32 or 64 bits.

BrunoBelucci avatar Feb 15 '23 14:02 BrunoBelucci

After some thought, I decided to change np.float to np.float32 following the assumption that we usually work with GPUs, so eps from np.float64 can be 0 if we use np.float32, but eps from np.float32 will never be 0.

BrunoBelucci avatar Mar 07 '23 16:03 BrunoBelucci

Is this update still in progress?

EthanReid avatar Nov 21 '23 18:11 EthanReid