Skip to content

BUG: For integer arguments, round returns a view while ceil returns a copy #29124

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.

Sign up for GitHub

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

Jump to bottom
Open
ev-br opened this issue Jun 4, 2025 · 5 comments · May be fixed by #29137
Open

BUG: For integer arguments, round returns a view while ceil returns a copy #29124

ev-br opened this issue Jun 4, 2025 · 5 comments · May be fixed by #29137
Labels
00 - Bug

Comments

@ev-br
Copy link
Contributor

ev-br commented Jun 4, 2025

Describe the issue:

I expected that all of ceil, trunc and round always return either a view or a copy for all dtypes. However, for integer arguments, round returns a view for integer inputs and a copy for float inputs.
ceil and floor all return a copy for both integers and floats.

IIUC the return dtype was changed in numpy 2.0, so this looks like a small omission.

Reproduce the code example:

In [13]: for func in [np.round, np.ceil, np.trunc]:
    ...:     for dt in [int, float]:
    ...:         a = np.arange(3, dtype=dt)
    ...:         b = func(a)
    ...:         is_view = np.shares_memory(a, b)
    ...:         print(f"{func.__name__}  {a.dtype} : {is_view}")
    ...: 
round  int64 : True
round  float64 : False
ceil  int64 : False
ceil  float64 : False
trunc  int64 : False
trunc  float64 : False

Error message:

.

Python and NumPy Versions:

In [5]: np.__version__
Out[5]: '2.4.0.dev0+git20250603.ff1d6cc'

Runtime Environment:

In [14]: np.show_runtime()
WARNING: `threadpoolctl` not found in system! Install it by `pip install threadpoolctl`. Once installed, try `np.show_runtime` again for more detailed build information
[{'numpy_version': '2.4.0.dev0+git20250603.ff1d6cc',
  'python': '3.12.10 | packaged by conda-forge | (main, Apr 10 2025, 22:21:13) '
            '[GCC 13.3.0]',
  'uname': uname_result(system='Linux', node='gonzales', release='5.15.0-76-generic', version='#83~20.04.1-Ubuntu SMP Wed Jun 21 20:23:31 UTC 2023', machine='x86_64')},
 {'simd_extensions': {'baseline': ['SSE', 'SSE2', 'SSE3'],
                      'found': ['SSSE3',
                                'SSE41',
                                'POPCNT',
                                'SSE42',
                                'AVX',
                                'F16C',
                                'FMA3',
                                'AVX2'],
                      'not_found': ['AVX512F',
                                    'AVX512CD',
                                    'AVX512_KNL',
                                    'AVX512_KNM',
                                    'AVX512_SKX',
                                    'AVX512_CLX',
                                    'AVX512_CNL',
                                    'AVX512_ICL',
                                    'AVX512_SPR']}}]

Context for the issue:

Found in data-apis/array-api-compat#333 which aims to make sure that array_api_compat.numpy does not change the view/copy semantics of numpy itself.
@ev-br ev-br added the 00 - Bug label Jun 4, 2025
@seberg
Copy link
Member

seberg commented Jun 5, 2025

I didn't realize round returned views. Ufuncs never do, right now. I think they could for new-style ufuncs, but they don't in practice.
Don't have a strong opinion on whether it's worth pushing for a change here. Views are nice, but you do have the downside that you don't know whether the result is a view or not (and if out= is passed you have to do a copy anyway).

@ev-br
Copy link
Contributor Author

ev-br commented Jun 6, 2025

Views are nice, but you do have the downside that you don't know whether the result is a view or not

Which is exactly the issue with round: on main you don't know whether the result is a view or a copy. ceil, floor and trunc seem to always copy, and it'd be nice if round did the same.
(I mean, if y'all want to change them all to always return views, the user in me is equally happy--as long as they are all consistent and dtype-independent).

@seberg
Copy link
Member

seberg commented Jun 6, 2025 •
edited
Loading

Ah, yeah, I would feel more safe just returning a copy for round always, not sure if I think it is particularly important to change, but I would have no objection to doing so.

@ev-br ev-br linked a pull request Jun 7, 2025 that will close this issue
Open
@rgommers
Copy link
Member

rgommers commented Jun 8, 2025

I agree this change is good to make. In principle, any change from view to copy or vice versa is a backwards compatibility break, but it this case it seems unlikely that it's going to bite anyone unless their code is already buggy. This looks like a bug rather than an intentional design choice.

IIUC the return dtype was changed in numpy 2.0, so this looks like a small omission.

@ev-br can you clarify what you mean here? I can interpret it as "the behavior in 1.2x was to always return a copy", or "this was changed for other functions in 2.0 and round was overlooked", or ...

@ev-br
Copy link
Contributor Author

ev-br commented Jun 8, 2025

can you clarify what you mean here? I can interpret it as "the behavior in 1.2x was to always return a copy", or "this was changed for other functions in 2.0 and round was overlooked", or ...

Meant the latter, sorry for not being clear. Consider

>>> np.__version__
'1.22.0'
>>> np.round(np.arange(3)).dtype
dtype('int64')
>>> np.floor(np.arange(3)).dtype    # ceil and trunc are similar
dtype('float64')               # ouch

So floor, ceil and trunc changed the output dtype to match round:

>>> np.__version__
'2.1.3'
>>> np.round(np.arange(3)).dtype
dtype('int64')
>>> np.floor(np.arange(3)).dtype
dtype('int64')

That the output dtype is now consistent is great. A small hiccup which this PR aims to fix is that round returns a view while other "rounding-like" functions return a copy. The behavior of round traces all the way back to 2009 or earlier, as the last git blame entry for the line this PR changes is 41d4199.

The resulting inconsistency is probably not easy to find unless you're unlucky to hit it (and then it can be a bit of a chore to debug), or are specifically looking for it :-). The only reason I found it now was that I mistyped round when meaning trunc in data-apis/array-api-compat#333.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
00 - Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

BUG: make round consistently return a copy ev-br/numpy
3 participants
@seberg @rgommers @ev-br