-
Notifications
You must be signed in to change notification settings - Fork 92
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
The WriteVTK wrapper: Multiblockfiles and VTKFileCollection #1004
Comments
Adding 1.0 since removing Also pointing the comments where the multiblock was discussed in #692 since the discussion is so long there. |
Just thinking out loud for some options 1) Introduce wrappers for the collection typesusing Ferrite
pvd = VTMFileCollection(filename)
...
addstep!(pvd, t) do vtm # ::VTMFile
for part in parts
VTKFile(vtm, partname, part.grid) do vtk
write_solution(vtk, dh, u)
write_...
end
end
end
# Without `addstep!`
pvd = VTMFileCollection(filename)
...
vtm = VTMFile("my_vtm_file")
for part in parts
VTKFile(vtm, partname, part.grid) do vtk
write_solution(vtk, dh, u)
write_...
end
end
pvd[state.t] = vtm 2) Remove
|
Independent of the result in the discussion on this specific format, I think we should provide a guide on how to extend the writers.
That would be a FerriteDistributed.jl issue I think.
This does look different from the current design. I thought we agree on using |
Yes, we could do Right! we should then have However, a
yes - just wanted to keep that in mind :) |
Why? A multiblock is literally a type of VTK file. It is also modeled this way in WriteVTK (https://juliavtk.github.io/WriteVTK.jl/stable/API/#WriteVTK.MultiblockFile). |
Maybe I got it wrong, but I read "Multiblock files (.vtm) are XML VTK files that can point to multiple other VTK files.", and interpreted this as a collection of vtk files similar to pvd. Especially since it is listed under "Metadata formats". And the point of not putting a multiblock in the So my logic is that |
Correct. I have said nothing else. :)
Not sure if this is the right interface anyway. It should be |
Yes,
|
I see. We should change the name then, as I think this is confusing on two levels. I think the naming here is a bit unlucky, because our VTK file is not really a wrapper for VTK files but for VTU files, right? Furthermore we can introduce some supertype for vtk data files. This would also fit with the proposal in this discussion, that we can add some |
It wraps I think changing the name might be a good idea, seeing this confusion! While
I don't see what the common interface would be here? The other vtk file collect multiple IMO, a supertype should rather have subtypes that support the |
And the more we discuss this, the clearer it is to me that it was probably a mistake to introduce the |
👍
Sorry I cannot reconstruct my train of thought how I arrived at the conclusion. Will post if I catch it again.
Not sure about this. Maybe we should put some thought into making this more flexible? |
I'm not sure how, but happy to get suggestions! This is quite urgent due to 1.0, so learning from the previous mistakes, I think the most important right now is to keep the 0.3.14 features while promising as little as possible - we can always provide something nicer in the future. For now, I'm fixing the naming first in #1014 - there I also realized now that we had used |
So if I understand correctly, the alternatives are
Maybe 1) is a bit more future proof, where we can decide on an "export" interface in the future? |
Yes, see my comments in #1015 for how I see that option one could still be quite user-friendly in the future, while keeping the 0.3.14 behavior for now. |
Before the introduction of the WriteVTK wrapper in #692 , it was easier to work with multiblock files and paraview collection files (to save multiple time steps). From one of my packages:
This has changed now:
VTKFileCollection(name, grid_or_dh)
), which is not always known at the construction of the paraview collection. We should either make it possible to havegrid_or_dh===nothing
or remove it completely (depending on if we only want to have a thin wrapper or extend it with our own functionality).The text was updated successfully, but these errors were encountered: