Skip to content

Fix for #2358 - #2360

Merged
akshaysridhar merged 1 commit into
mainfrom
rc/i2358
Aug 14, 2025
Merged

Fix for #2358#2360
akshaysridhar merged 1 commit into
mainfrom
rc/i2358

Conversation

@ray-chew

@ray-chew ray-chew commented Aug 1, 2025

Copy link
Copy Markdown
Member

Fix for the quadrature_style not working for 2D horizontal slices

Comment thread src/Grids/level.jl Outdated
@ray-chew
ray-chew force-pushed the rc/i2358 branch 6 times, most recently from e7a1439 to 5f6c3b6 Compare August 12, 2025 06:34
@ray-chew

Copy link
Copy Markdown
Member Author

Thanks you two and @dennisYatunin. So the tests run now and are passing CI. But the formatter keeps failing, and I don't know why. The files I changed passed my formatter locally:
using JuliaFormatter; format("test/InputOutput/unit_hybrid3dcubedsphere_topography.jl")

A little more detail on the tests:

  • A test HDF5 restart test for a Level of a 3D hybrid cubed sphere for deep on saving and loading Levels is included in test/InputOutput/unit_hybrid3dcubedsphere_topography.jl. This feature is used in the preprocessing of the topography.
  • Following Dennis' advice, I moved the test for the gradh of Levels to Levels of nonlocal Fields and nonlocal Field broadcasts in test/Fields/unit_field.jl.
  • The levels.jl and readers.jl were modified to fix the issue with the quadrature style.

@akshaysridhar

akshaysridhar commented Aug 12, 2025

Copy link
Copy Markdown
Member

Thanks for adding the tests @ray-chew : The formatter fails checks against 3 files :

test/Fields/unit_field.jl
test/InputOutput/unit_hybrid3dcubedsphere_topography.jl
test/Operators/hybrid/extruded_3dbox_cuda.jl

Try applying the format change to each of these, then I think we are ready to merge this PR.

@ray-chew

Copy link
Copy Markdown
Member Author

Thanks for adding the tests @ray-chew : The formatter fails checks against 3 files :

test/Fields/unit_field.jl
test/InputOutput/unit_hybrid3dcubedsphere_topography.jl
test/Operators/hybrid/extruded_3dbox_cuda.jl

Try applying the format change to each of these, then I think we are ready to merge this PR.

That's what I did, as mentioned in the comment above your reply:

using JuliaFormatter; format("test/InputOutput/unit_hybrid3dcubedsphere_topography.jl")
using JuliaFormatter; format("test/Fields/unit_field.jl")
using JuliaFormatter; format("test/Operators/hybrid/extruded_3dbox_cuda.jl")

The return values for all three were true, and I did the commands above at least a dozen times.

@nefrathenrici

Copy link
Copy Markdown
Member

From the top-level directory, you should be able to run julia -e 'using JuliaFormatter; format(".")' to format the entire repo. I also recommend ensuring you are using JuliaFormatter 1.0.62 as well.

@ray-chew

Copy link
Copy Markdown
Member Author

From the top-level directory, you should be able to run julia -e 'using JuliaFormatter; format(".")' to format the entire repo. I also recommend ensuring you are using JuliaFormatter 1.0.62 as well.

Oh ok, my JuliaFormatter version is 2.1.6. That's the issue. If I ran 2.1.6. on the whole repository like you suggested, it formats a whole bunch of files (like 30 something). @akshaysridhar reproduced it with JuliaFormatter v2.1.6 as well. Thanks for your help, you two!

@ray-chew
ray-chew force-pushed the rc/i2358 branch 3 times, most recently from 92c7bc3 to 84ec899 Compare August 13, 2025 00:06
@ray-chew

Copy link
Copy Markdown
Member Author

Ok, so the CIs are finally passing. Can someone (@nefrathenrici @akshaysridhar) please review and merge these? Thanks!

Yesterday, one of the GPU tests that was working failed all of a sudden. But then it started working again last night, lol. I don't know what happened, as I didn't touch those tests. But just wanted to document this for posterity.

@akshaysridhar

akshaysridhar commented Aug 14, 2025

Copy link
Copy Markdown
Member

@nefrathenrici The latest commit seems reasonable (much simpler solution than what the original commits had, no need to include the additional property since LevelGrid inherits the full_grid properties.)

@akshaysridhar
akshaysridhar merged commit a078484 into main Aug 14, 2025
35 checks passed
@akshaysridhar
akshaysridhar deleted the rc/i2358 branch August 14, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CUDA GPU InvalidIRError when using horizontal gradient on a LevelGrid

3 participants