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

Sort out naming in XDMFFile method names #20

Closed
garth-wells opened this issue Apr 10, 2018 · 7 comments
Closed

Sort out naming in XDMFFile method names #20

garth-wells opened this issue Apr 10, 2018 · 7 comments
Assignees

Comments

@garth-wells
Copy link
Member

garth-wells commented Apr 10, 2018

From @blechta

It has been agreed in a discussion that following renaming scheme is favourable

class XDMFFile:
    read_checkpoint() -> read()
    write_checkpoint() -> write()
    write() -> write_vertex_values()

Arguably write_vertex_values() could be removed once support for write_checkpoint() format in Paraview is packaged and distributed well. User can instead do more explicit operation involving some kind of interpolation or projection.

See https://bitbucket.org/fenics-project/dolfinx/issues/27/sort-out-naming-in-xdmffile for comments.

@garth-wells garth-wells changed the title Sort out naming in XDMFFile Sort out naming in XDMFFile method names Apr 20, 2018
@michalhabera michalhabera self-assigned this May 4, 2018
@michalhabera
Copy link
Contributor

Addressed in https://github.com/FEniCS/dolfinx/tree/michal/sort-naming-xdmf
Instead of read_checkpoint -> read I applied read_checkpoint -> read_function to stay consistent with the rest of XDMF, where write is implicit and read is explicit (argument type in name).

But I don't like the wrapping in python/dolfin/io.py ... Did you guys (@garth-wells @chrisrichardson ) found a better way for this dolfin.function.function.Function casting?

@garth-wells
Copy link
Member Author

garth-wells commented May 4, 2018

@michalhabera I'm tending toward nothing in the cpp namespace being 'public'. All Python functionality would be implemented in Python, with calls to the backend. In this case we would not have to do any casting in the pybind11 code.

This would also help with documentation. The Python and C++ interface would have their own docs.

@michalhabera
Copy link
Contributor

Thanks for clarification, @garth-wells . If I got it right it means, that python docstrings would go to python/dolfin/ files.
Disadvantage of course is that for any interface change one must change C++ code and docs, its "private" pybind11 cpp.* wrappers and its "public" python wrappers with docstrings...
But that is a lot of work to wrap everything in that additional python layer, code that has to be maintained... Or do you want to "wrap" by inheriting the cpp.* classes?

@garth-wells
Copy link
Member Author

Some will just be import foo as bar.

I don't expect it will be more work that what we already have, and likely less . In cases it will be less because we won't have to manage something one on of two things, e.g. a dolfin.function.Function or a dolfin.cpp.function.Function.

@michalhabera
Copy link
Contributor

More work has to be done. Renaming scheme from above is misleading (see #62 ).

Renaming write_checkpoint -> write and read_checkpoint -> read_function is agreed since no hidden things are going on, function is saved as is.

On the other hand VTKish format in XDMFFile should be more explicit. The idea is to have
a.) write_cell_data which interpolates function to P0 and mesh is left as is,
b.) write_p1_point_data which interpolates to isoparametric P1 (mesh and function),
b.) write_p2_point_data which interpolates to isoparametric P2 (mesh and function).

The same names would be applied also to VTKFile. Any comments appreciated before I start the work.

@michalhabera
Copy link
Contributor

The naming above is too complicated. We need to keep things easy.
With the proper and fast interpolation (#161) most of the under-the-hood interpolation should happen explicitly by user action.

And in order to avoid new function memory allocation just to IO, we can have some local, cell-wise interpolation methods. These can work as iterators and cell by cell flush the values.

@jorgensd
Copy link
Member

@garth-wells this is not resolved. Right now we have removed write_checkpoint, and we have to reintroduce it at some point.

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

No branches or pull requests

3 participants