Skip to content

Conversation

ray-chew
Copy link
Member

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

Fix for the quadrature_style not working for 2D horizontal slices

@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
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
Copy link
Member

akshaysridhar commented Aug 12, 2025

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
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
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
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
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
Copy link
Member

akshaysridhar commented Aug 14, 2025

@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