JuliaVTK / ReadVTK.jl

Julia package for reading VTK XML files
https://juliavtk.github.io/ReadVTK.jl
MIT License
31 stars 10 forks source link

add support for uniform structured grids #9

Closed jorgepz closed 2 years ago

jorgepz commented 2 years ago

closes #8

for debugging I am using this MWE

# ReadVTK.jl gridExample
using WriteVTK, ReadVTK

x, y, z = 0:10, 1:6, 2:0.1:3
Nx, Ny, Nz = length(x), length(y), length(z)

scalarField = rand(Nx, Ny, Nz)

vtk_grid("grid", x, y, z) do vtk
    vtk["Temperature", VTKPointData()] = scalarField              # scalar field attached to points
end

filepath = "grid.vti"
vtk = VTKFile( filepath )
jorgepz commented 2 years ago

I have a first MWE where cell data is well read and a validation is done. the changes in readvtk are intentionally minimal in order to avoid conflicts. the test example i am running (and by the moment I did not include in the repo) is the following


# ReadVTK.jl gridExample
using WriteVTK, ReadVTK

## Generate grid file and write vti
x, y, z = 1:3, 1:2, 2:0.1:2.2
Nx, Ny, Nz = length(x), length(y), length(z)

pointScalarField = rand(Nx, Ny, Nz)
cellScalarField  = rand(Nx-1, Ny-1, Nz-1)

print( "cell scalar field", cellScalarField,"\n")

cellDataName  = "Name of Test Cell scalar data"
pointDataName = "Point scalar data"

print("  writing vtk file...")
vtk_grid("grid", x, y, z) do vtk
    vtk[ pointDataName, VTKPointData()] = pointScalarField  # scalar field attached to points
    vtk[ cellDataName, VTKCellData()] = cellScalarField     # scalar field attached to cells
end
print("done.\n")

## Read vti file
print("  reading vtk file...")
filepath = "grid.vti"
vtk = VTKFile( filepath )
data = get_data( get_cell_data(vtk)[cellDataName] )
print("done.\n")

reshapedData = reshape( cellScalarField, ( (Nx-1), (Ny-1), (Nz-1) ) )

difference = reshapedData .- cellScalarField

the difference array is indeed empty

ranocha commented 2 years ago

Thanks a lot! I didn't dive through the code, but I would be fine with adding WriteVTK.jl as test dependency. This would make it much simpler to extend things in the future and make sure that the Julia VTK packages are compatible with each other.

sloede commented 2 years ago

but I would be fine with adding WriteVTK.jl as test dependenc

Absolutely! It would be great if you can add some tests, maybe along the lines of what you gave as an MWE.

The implementation looks good so far as well. Some things to consider:

jorgepz commented 2 years ago

Hi, thanks both for your comments!

In c75860d I included my small test script in the runtest.jl script and it seems to be working. I also added the dependency to the tests Toml as @ranocha suggested, however I am not experienced (I am a Julia newbie) in using diifferent toml's, so in order to avoid an error I am activating from the root project and including the runtest.jl from there (with WriteVTK) installed. A documentation entry for the right approach for the testing would be nice too I think, but I would not do that in this PR to focus. By the moment I am in my root folder opening julia with julia --project=. and from there using Revise and include("test/runtests.jl"). That works well (if I have installed writevtk).

Regarding the questions from @sloede

jorgepz commented 2 years ago

In 3e7683a I added a small get_origin function and also a test for that. I also separated that in another file, as well as moved the exports to another file.. please let me know if you prefer to keep everything in a single file... if you agree on this approach I could continue with the other fields like: extent, spacing and point data. Also, I think that running the CI workflows would be good at this point, so that we can detect other OS problems... (I am working in Arch Linux).

ranocha commented 2 years ago

in order to avoid an error I am activating from the root project and including the runtest.jl from there (with WriteVTK) installed. A documentation entry for the right approach for the testing would be nice too I think, but I would not do that in this PR to focus. By the moment I am in my root folder opening julia with julia --project=. and from there using Revise and include("test/runtests.jl"). That works well (if I have installed writevtk).

That sounds reasonable. Alternatively, you could also activate the main environment of ReadVTK.jl and run

using Pkg; Pkg.test("ReadVTK")

to run all tests of ReadVTK.jl. Then, the Julia package manager will take care of installing WriteVTK (in a temporary environment for running the tests).

coveralls commented 2 years ago

Coverage Status

Coverage decreased (-0.7%) to 92.079% when pulling e3fe8e1728a33377d6e8f2a471c5b214f24005c6 on jorgepz:main into 04832aba0904359eb20152c120d8212dec8bed73 on trixi-framework:main.

jorgepz commented 2 years ago

Thanks for your comments @sloede!

  • Have you tried using it for 2D files as well? Should we maybe add a test for a 2D file?

I did not try to use 2D files, since in the applications I am planning to use ReadVTK and WriteVTK everything is computed in a general 3D manner (even for 2D or 1D grids). I do not know the interval vtk structure to know if it stores data in a different way. I would guess that it does not.

  • Have you tried using it for non-scalar datasets, e.g., vectorial or tensor fields?

This is a useful feature that I will be needing in the future, and I would definitely add a test for this in the following PR. By the moment I thought about starting with scalar and see how I went...

To have in mind the applications I am planning to solve: I'll be using WriteVTK and ReadVTK to generate images or 3D solids during deformation to solve these kind of problems https://www.sciencedirect.com/science/article/abs/pii/S0045782519302099

sloede commented 2 years ago

One further note @jorgepz: I think you should add your name to the authors' list. For example, you could add

Further contributions to ReadVTK have been made by the following people:
* Jorge Pérez Zerpa

after https://github.com/jorgepz/ReadVTK.jl/blob/fae422c9d4f126835a8ca9cf4f77903b6b35ca4e/README.md?plain=1#L103

and https://github.com/jorgepz/ReadVTK.jl/blob/fae422c9d4f126835a8ca9cf4f77903b6b35ca4e/docs/src/index.md?plain=1#L97

jorgepz commented 2 years ago

One further note @jorgepz: I think you should add your name to the authors' list. For example, you could add

Thanks @sloede . I did a proposal in d95f37b

sloede commented 2 years ago

@jorgepz Unfortunately, there is an error in the Windows tests: https://github.com/trixi-framework/ReadVTK.jl/runs/6833929733?check_suite_focus=true

jorgepz commented 2 years ago

By what I see in the tests, the windows version was working until the Int change in 826a1f3 ... strange... I could test it locally on windows 11 later...

jorgepz commented 2 years ago

Yes thanks @sloede it seems it was working here https://github.com/trixi-framework/ReadVTK.jl/runs/6833787929?check_suite_focus=true ... will check later. thanks.

sloede commented 2 years ago

The Windows error disappeared after retriggering the tests. Let's hope that they stay away.