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.

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

Do we allow scalar indexing in all our tests? #3039

Open
simone-silvestri opened this issue Mar 30, 2023 · 4 comments · May be fixed by #4039
Open

Do we allow scalar indexing in all our tests? #3039

simone-silvestri opened this issue Mar 30, 2023 · 4 comments · May be fixed by #4039
Labels
GPU 👾 Where Oceananigans gets its powers from testing 🧪 Tests get priority in case of emergency evacuation

Comments

@simone-silvestri
Copy link
Collaborator

simone-silvestri commented Mar 30, 2023 •
edited
Loading

Oceananigans.jl/test/runtests.jl

Line 3 in bcc34f0

CUDA.allowscalar() do

We should remove this
@navidcy
Copy link
Collaborator

navidcy commented Mar 31, 2023

We should remove this and only do that where is absolutely necessary. Ideally nowhere!

@navidcy navidcy added the GPU 👾 Where Oceananigans gets its powers from label Mar 31, 2023
@navidcy
Copy link
Collaborator

navidcy commented Mar 31, 2023

I made an attempt in #2701 to put the allowscalar in the places that the tests required them and remove the general allowscalar statement that @simone-silvestri points out above. But I couldn't finish the attempt... I was getting so many tests failing and I admit I got worn out in the end and stopped that effort......

@navidcy navidcy added the testing 🧪 Tests get priority in case of emergency evacuation label Apr 4, 2024
@navidcy
Copy link
Collaborator

navidcy commented Apr 4, 2024

bumping this up

I'd like to remove the generic CUDA.allowscalar() from tests!

cc @glwagner, @simone-silvestri, @siddharthabishnu

@glwagner
Copy link
Member

glwagner commented Apr 4, 2024

Try it?

@simone-silvestri simone-silvestri linked a pull request Jan 12, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
GPU 👾 Where Oceananigans gets its powers from testing 🧪 Tests get priority in case of emergency evacuation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove global allowscalar from testing suite attempt 2 CliMA/Oceananigans.jl
3 participants
@navidcy @glwagner @simone-silvestri