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.

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

Read vtk #657

Draft
wants to merge 22 commits into
base: dev
Choose a base branch
from
Draft

Read vtk #657

wants to merge 22 commits into from

Conversation

LasNikas
Copy link
Collaborator

No description provided.

Copy link

codecov bot commented Nov 10, 2024

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.

Project coverage is 68.74%. Comparing base (550b95b) to head (6710a12).

Files with missing lines Patch % Lines
src/preprocessing/readvtk/vtk2trixi.jl 0.00% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #657      +/-   ##
==========================================
- Coverage   68.88%   68.74%   -0.15%     
==========================================
  Files          86       87       +1     
  Lines        5261     5272      +11     
==========================================
  Hits         3624     3624              
- Misses       1637     1648      +11     
Flag Coverage Δ
unit 68.74% <0.00%> (-0.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator Author

@LasNikas LasNikas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work so far. Thanks a lot!
I have some initial remarks. Also, what about the files in examples/vtk-reader? Did you use these files for development? If so, and these files are not important, please remove them.
Tests are missing.

@@ -0,0 +1,36 @@
using ReadVTK
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put this in src/TrixiParticles.jl between line 20 and 21. Packages are sorted alphabetically.


# FluidSystem data only
"""

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

Comment on lines 4 to 6
Convert data from VTK-file to InitialCondition

# FluidSystem data only
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either add a TODO or elaborate more how to use the function. Look at other docs for inspiration.

# FluidSystem data only
"""

function vtk2trixi(filename)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests are missing

velocity = vcat(velocity, zeros(1, size(velocity, 2)))
end

mass = ones(size(coordinates, 2))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create an issue that we need to write the mass per particle in trixi2vtk.

Project.toml Outdated
RecipesBase = "3cdcf5f2-1ef4-517c-9805-6587b60abb01"
Reexport = "189a3867-3050-52da-a836-e630ba90ab69"
SciMLBase = "0bca4576-84f4-4d90-8ffe-ffa030f20462"
StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
StrideArrays = "d1fa6d79-ef01-42a6-86c9-f7c551f8593b"
TimerOutputs = "a759f4b9-e2f1-59dc-863e-4aeb61b1ea8f"
TrixiBase = "9a0f1c46-06d5-4909-a5a3-ce25d3fa3284"
VTKDataIO = "c6703add-1d23-52c6-9943-3ad88652b9b2"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
VTKDataIO = "c6703add-1d23-52c6-9943-3ad88652b9b2"

Please undo all this changes in the Project.toml except ReadVTK.

using TrixiParticles
using OrdinaryDiffEq

ic = vtk2trixi("examples/readvtk/fluid_1_1.vtu")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use joinpath() for this. Also it is better to store an example vtk file in examples/reprocessing/data and name it read_vtk_example.vtu or so.

# FluidSystem data only
"""

function vtk2trixi(filename)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this consistent with trixi2vtk. That is, please use the kwargs input_directory="out", prefix="", iter=nothing.
IMO, this makes it easier to later find the right file.

Comment on lines 24 to 27
if size(velocity, 1) == 2
# If velocity is 2D, add 0.0 for the z component
velocity = vcat(velocity, zeros(1, size(velocity, 2)))
end
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if size(velocity, 1) == 2
# If velocity is 2D, add 0.0 for the z component
velocity = vcat(velocity, zeros(1, size(velocity, 2)))
end
# Retrieve particle coordinates.
coordinates = get_points(vtk_file)[axes(velocity, 1), :]

Also, please add to the comment that the point coordinates are stored in 3D coordinates, but the velocity can be either 2D or 3D. I think this has something to do with the vtk data structure. You might want to read up on it.

Comment on lines 12 to 13
# Retrieve particle coordinates
coordinates = get_points(vtk_file)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Retrieve particle coordinates
coordinates = get_points(vtk_file)

Project.toml Outdated
CSV = "336ed68f-0bac-5ca0-87d4-7b16caf5d00b"
DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"
DelimitedFiles = "8bb1440f-4735-579b-a4ab-409b98df4dab"
DiffEqCallbacks = "459566f4-90b8-5000-8ac3-15dfb0a30def"
Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"

Project.toml Outdated
@@ -18,17 +20,20 @@ JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6"
KernelAbstractions = "63c18a36-062a-441e-b654-da1e3ab1ce7c"
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
MuladdMacro = "46d2c3a1-f734-5fdb-9937-b9b9aeba4221"
OrdinaryDiffEq = "1dea7af3-3e70-54e6-95c3-0bf5283fa5ed"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
OrdinaryDiffEq = "1dea7af3-3e70-54e6-95c3-0bf5283fa5ed"

marcelschurer and others added 17 commits November 15, 2024 12:06
* Fix callback `show` and remove hacky workaround

* Reforamt code

* Fix tests

* Make it work for all versions
* set test up for 1.11

* add basic struct

* add export

* make equation available

* format

* typo

* fix docs

* add exmaple

* format

* fix

* Increase errors for 1.11

* Fix invalidations

* Fix tests

* Update ci.yml

* revert

* Update ci.yml

* Update test/validation/validation.jl

Co-authored-by: Erik Faulhaber <[email protected]>

* format

* review fixes

* add test

* fix example

* fix test

* review comments

* fix docs

* Update src/schemes/fluid/weakly_compressible_sph/state_equations.jl

Co-authored-by: Erik Faulhaber <[email protected]>

* forgot to edit the other doc

---------

Co-authored-by: Erik Faulhaber <[email protected]>
* Add tutorial for setting up a simulation

* Add section about custom smoothing kernels

* Implement suggestions

* Add kernel plot

* Try to remove warnings

* Implement suggestions

* Make example a dam break and plot initial condition

* Fix rendering

* Fix tank size

* Fix range for kernel plot

* Fix descriptions after changing the example

* Add equation for kernel

* Fix minor errors
* add `density_calculator` to example

* add perturbation option

* fix typo

* implement suggestions

---------

Co-authored-by: Sven Berger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants