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

Numpy 2.0 compatibility #251

Merged
merged 4 commits into from
Apr 12, 2024
Merged

Numpy 2.0 compatibility #251

merged 4 commits into from
Apr 12, 2024

Conversation

saitcakmak
Copy link
Contributor

While reviewing pytorch/botorch#2293, I noticed test failures coming from cma due to incompatibility with Numpy 2.0. One thing led to another and here we are with a PR making cma compatible with Numpy 2.0.

Changelist:

  • Replace array(…, copy=False) with np.asarray(…)
  • Replace np.Inf with np.inf
  • Replace np.alltrue with np.all
  • Replace np.NaN with np.nan

Test fixes:

  • list(np.ndarray) returns [np.float64(value), …] rather than [value, …]. So, using np.ndarray.tolist() instead.
  • Similarly, functions return np.float64(value), which are compared to native float values.
  • Add .item() calls to approx_equal to make it return native True rather than np.True_

Test plan:

  • pip install numpy==2.0.0rc1
  • python -m cma.test

@@ -1029,7 +1029,7 @@ def pheno(self, x, into_bounds=None, copy=True,
input_type = type(x)
if into_bounds is None:
into_bounds = (lambda x, copy=False:
x if not copy else array(x, copy=copy))
x if not copy else np.asarray(x, copy=copy))
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a bug: np.asarray doesn't accept a copy argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! I'll change this to array(x, copy=True) to make it clear that it's only called when copy=True.

Copy link
Contributor

@nikohansen nikohansen left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Looks good beside the small point I commented on.

Another question: did you consider using float(.) instead of .item() when a np.float64 is expected? I guess both has pros and cons.

@saitcakmak
Copy link
Contributor Author

Another question: did you consider using float(.) instead of .item() when a np.float64 is expected? I guess both has pros and cons.

I haven't thought of using float(.), just went with .item() since that's what I am used to using with numpy & pytorch. I guess it wouldn't make a meaningful difference since the changes are contained to doctests -- though I don't really know how different the two are under the hood.

@nikohansen nikohansen merged commit 614f229 into CMA-ES:development Apr 12, 2024
2 checks passed
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.

None yet

2 participants