Skip to content

Conversation

jrper
Copy link
Contributor

@jrper jrper commented Nov 4, 2019

For VTK 8.2+ and Paraview 5.5+ this automagically gets used to set the time levels of a file series.

q.v. https://gitlab.kitware.com/vtk/vtk/commit/6e0acf3b773f120c3b8319c4078a4eac9ed31ce1

I'm raising this relatively early, since I suspect there may be some discussion of the right thing to do about this, and whether this should be default on or default off, as well as how to test it.

For VTK 8.2+ and Paraview 5.5+ this automagically gets used to set the time levels of a file series.
Copy link
Contributor

@stephankramer stephankramer left a comment

Choose a reason for hiding this comment

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

Other than that, this looks good.

A further suggestion: if this still produces valid vtk files that can also be read in in paraview < 5.5, should we just make this the default? In which case I would suggest to have no option at all (we have too many already!). Or does this change paraview behaviour in > 5.5 in a way that people might not always want? (haven't tried it myself)

Did you try this in parallel?

@@ -238,6 +241,7 @@ subroutine vtk_write_fields(filename, index, position, model, sfields,&
type(tensor_field), dimension(:), intent(in), optional :: tfields
logical, intent(in), optional :: write_region_ids
logical, intent(in), optional :: write_columns
type(scalar_field), dimension(:), optional :: metadata
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, this seems to abuse scalar_field as a container for a real array (with a name). A scalar_field is defined on a mesh (function space!), and it suggests we care which mesh. I can see this is a compact way of supplying one or more real arrays of different lengths with names, but if people want to use this in a generic way they really will have to abuse scalar_fields by allocating a %val array that is inconsistent with whatever random mesh they choose to use. Also when first reading this code, I thought it was just writing out an extra field (i.e an actual spatial field), so I think it's better to make it explcit that we're just supplying a real array of arbitrary length.

Since we only use this for time at the moment, I would be happy to just have a real, intent(in):: time_value argument (where the name is hardcoded to TimeValue). Alternatively you could use an array of real_vectors (see femtools/Futils.F90) with a separate array of names.

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.

2 participants