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 for issue 7369 #7514

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

westford14
Copy link

@westford14 westford14 commented Sep 25, 2024

Fix for numpy deprecation warning (#7369)

Description

Small PR that addresses the numpy deprecation warnings that are seen when running tests:

tests/step_methods/test_metropolis.py: 15950 warnings
  REDACTED/pymc/pymc/backends/ndarray.py:118: DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.)
    data[key][self.draw_idx] = val

Change looks at the type of the val and if it is a np.ndarray and has .shape[0] == 1, then we pop the item out to prevent the deprecation warning.

Related Issue

Checklist

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pymc--7514.org.readthedocs.build/en/7514/

Copy link

welcome bot commented Sep 25, 2024

Thank You Banner]
💖 Thanks for opening this pull request! 💖 The PyMC community really appreciates your time and effort to contribute to the project. Please make sure you have read our Contributing Guidelines and filled in our pull request template to the best of your ability.

@@ -115,6 +115,8 @@ def record(self, point, sampler_stats=None) -> None:
if sampler_stats is not None:
for data, vars in zip(self._stats, sampler_stats):
for key, val in vars.items():
if isinstance(val, np.ndarray) and val.shape[0] == 1:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can val have more than one dim? More importantly it would be good if we didn't need this check in every iteration/ for every stat.

Can we solve it from another angle?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

none of the tests passed in any values of greater than one dimension -- but noted on the point about trying to solve this outside the loop!

@westford14
Copy link
Author

@ricardoV94 -- spent a little time looking at this and i think this problem is hard to solve outside the loop context given the setup of the sampler_stats object. At runtime we don't know what it's going to contain so we have to at some point loop through the values and look for np.ndarrays. In reference to your point about it being only one item, i think this is the case as this error only occurs when we're converting a 1-d array directly to a scalar, if the val isn't one dimensional we won't see this deprecation warning so we're fine to just check if it's just one value and, if it's not, continue

let me know what you think

@ricardoV94
Copy link
Member

ricardoV94 commented Sep 26, 2024

What happens if val has shape==(1, 2)? Your code will try to cerce into a scalar and fail right?

Taking a step back, the problem here seems to be the buffer was allocated with the wrong shape? If a stat has shape (1,) in every draw, the buffer should have (draws, 1) shape, not (draws,) and no coercing to scalar would be needed.

Am I off track here?

@westford14
Copy link
Author

What happens if val has shape==(1, 2)? Your code will try to cerce into a scalar and fail right?

Taking a step back, the problem here seems to be the buffer was allocated with the wrong shape? If a stat has shape (1,) in every draw, the buffer should have (draws, 1) shape, not (draws,) and no coercing to scalar would be needed.

Am I off track here?

okay yea that makes sense -- i'm not sure if there's a better way to coerce here than trying to find arrays of shape (1,) because i don't think it makes sense to traverse the sampler_stats prior to this inner loop

@ricardoV94
Copy link
Member

We should look at the code where the stats arrays are allocated to see how it's deciding on the shape

@ricardoV94
Copy link
Member

ricardoV94 commented Sep 30, 2024

The assumption that stats are scalars happens here:

data[varname] = np.zeros(draws, dtype=dtype)

If that's a requirement, then the problem is in Metropolis not respecting it. If that's not a requirement, the problem is in assuming the stats will be scalars. Which statistic from Metropolis step sampler is not respecting the scalar shape?

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 this pull request may close these issues.

Numpy deprecation warning issued from within test_metropolis.py
2 participants