Code review comment for lp:~asc/fluidity/regularisespatialaspect

Revision history for this message
Stephan Kramer (s-kramer) wrote :

Hm, I feel this option shouldn't be implemented inside vtk_write_fields(). vtk_write_fields is a low level routine that should just do what it's told to in the code, dump the fields that it's passed and not be dependent on options tree behaviour. Why don't you put this in vtk_write_state_new_option() and pass down the rescaled coordinate to vtk_write_fields() from there? That would also fix the checkpointing issue (which is a definite no-go for me).

I'm pondering how to make this option a bit more generally applicable:
- in general if I rescale my output for viewing (this is actually fairly trivial in paraview), I actually like to control the exact aspect ratio myself. Blowing it up to an exact 1:1 aspect ratio typically heavily distorts the features/mesh elements.
- it should also be made to work on the sphere, in which case it's hard to work out the rescaling parameter automatically
- if you have a free surface with mesh movement, automatically rescaling will cause the rescaling to be different every dump, which is probably not desired

Maybe have this option with a user specified rescaling? Or alternatively, just have a script that rescales the vtus afterwards. I'd like to hear other people's opinion on this...

« Back to merge proposal