Re: [Getfem-commits] please merge branch devel-tetsuo-xml

[Tetsuo Koyama]

I forgot to CC: in the last email, so I am re-sending it.
----------
Dear Kostas

Thank you for your reply.
> Thanks for your answer. Your code looks quite nice actually. I have one 
> question about the lines
>       std::vector<scalar_type> W(Q*pmf_dof_used.card());
>       gmm::copy(remove_dof_unused(V, pmf_dof_used, Q), W);
>       write_dataset_(V, name, qdim);
> Since you do not do anything with vector W, what is the meaning of having it? 
> Should the last line be:
>       write_dataset_(W, name, qdim);
> instead?
Yes you are right. Sorry my test was not enough. I fixed it.

> You can just make a new branch and put the outcome of your development in one 
> or two commits and then we can merge that branch. Sorry for being picky, but 
> establishing some good development habits will make our life easier in the 
> future.
Thanks. Your advice is very helpful to me. I made new branch
devel-tetsuo-xml02. It is a squash of commit of devel-tetsuo-xml.

Thank you for reading.

Best regards Tetsuo

>
> 2020年5月11日(月) 5:16 Konstantinos Poulios <address@hidden>:
> >
> > Dear Tetsuo,
> >
> > Thanks for your answer. Your code looks quite nice actually. I have one 
> > question about the lines
> >       std::vector<scalar_type> W(Q*pmf_dof_used.card());
> >       gmm::copy(remove_dof_unused(V, pmf_dof_used, Q), W);
> >       write_dataset_(V, name, qdim);
> > Since you do not do anything with vector W, what is the meaning of having 
> > it? Should the last line be:
> >       write_dataset_(W, name, qdim);
> > instead?
> >
> > Apart from that, I think we need a bit cleaner workflow without too many 
> > unnecessary commits. I remember that I had advised you in the past against 
> > too large commits, but the ideal is somewhere in the middle. The commits 
> > must in general be organized in logical units from the perspective of 
> > someone looking at the git history. The work you have done here, I would 
> > put it into one or two commits. All the forth and back during the 
> > development, it doesn't need to be part of the repository history.
> >
> > You can just make a new branch and put the outcome of your development in 
> > one or two commits and then we can merge that branch. Sorry for being 
> > picky, but establishing some good development habits will make our life 
> > easier in the future.
> >
> > Best regards
> > Kostas
> >
> > On Thu, May 7, 2020 at 3:23 PM Tetsuo Koyama <address@hidden> wrote:
> >>
> >> P.S.
> >> I had a typo
> >>
> >> >That is a good point. It maybe a good idea of using library, but we
> >> have to use cmake to link vtk librayr.
> >> That is a good point. It maybe a good idea of using library, but we
> >> have to use cmake to link vtk library.
> >>
> >> 2020年5月7日(木) 22:17 Tetsuo Koyama <address@hidden>:
> >> >
> >> > Dear Kostas
> >> >
> >> > Thank you very much for taking the time to review.
> >> >
> >> > > I think it is an important contribution to add vtu support, especially 
> >> > > if it is binary/compressed, just ascii is not very useful.
> >> > Thanks. I agree that binary/compressed is important. After this change
> >> > is confirmed, I would like to add that option.
> >> >
> >> > > However we might need to discuss a bit on how to do it. As far as I 
> >> > > can see you have used boost for xml writing. I think we
> >> > > had dropped our dependency on boost and I am not very keen on 
> >> > > reintroducing a dependency on boost.
> >> > I agree with the policy that projects don't use boost. In the end, I
> >> > made changes to eliminate the dependence on boost in the end. If there
> >> > is any remaining dependence, please point out . I am sorry that the
> >> > commit is complicated. The current vtu object does not require
> >> > dependency to boost even if when extending to binaries.
> >> >
> >> > >Before we merge this, I would like to hear some arguments for one 
> >> > >solution or another. The first thing to check is what others do.
> >> > > How is vtu export implemented in other software like e.g. fenics? What 
> >> > > is the more future-proof way of implementing vtu support?
> >> > I didn't search fenics but meshio package (This is a major package
> >> > which is used to convert mesh format. You can install by `apt install
> >> > python3-meshio`) and mayavi2.
> >> > Both are built by full scratches of writing text and binary like
> >> > getfem project is doing. They reffer vtk file format pdf.
> >> > https://vtk.org/wp-content/uploads/2015/04/file-formats.pdf
> >> >
> >> > > What is a solution with least dependencies? If we have to depend on an 
> >> > > external library it might be better to depend
> >> > > on vtk directly 
> >> > > https://www.paraview.org/Wiki/VTK/Examples/Cxx/IO/WriteVTU
> >> > That is a good point. It maybe a good idea of using library, but we
> >> > have to use cmake to link vtk librayr. I think it is difficult to use
> >> > with getfem using automake. (Is there any plan to use cmake in
> >> > getfem?)
> >> >
> >> > This is hello world of vtk library.
> >> > https://lorensen.github.io/VTKExamples/site/Cxx/GeometricObjects/CylinderExample/
> >> >
> >> > > Have you done some research regarding these questions?
> >> > That is all. If I need I search of fenics I will do it !
> >> >
> >> > > There is also another thing that I would like to ask you about. Could 
> >> > > you please don't use markup in your git commit description? It might 
> >> > > look nice in your git client but it looks ugly and difficult to read 
> >> > > on other's systems.
> >> > Thank you for pointing it out. I used emoji prefix which is popular in
> >> > my local. It is not good to use it in a global community. Sorry.
> >> >
> >> > > just to add that for compressed vtu files I use the attached 
> >> > > conversion script based on binary vtk files exported from getfem.
> >> > Thanks. I'll use it.
> >> >
> >> > Thank you for reading.
> >> >
> >> > BR Tetsuo
> >> >
> >> > 2020年5月7日(木) 19:21 Konstantinos Poulios <address@hidden>:
> >> > >
> >> > > just to add that for compressed vtu files I use the attached 
> >> > > conversion script based on binary vtk files exported from getfem.
> >> > >
> >> > > On Thu, May 7, 2020 at 11:59 AM Konstantinos Poulios <address@hidden> 
> >> > > wrote:
> >> > >>
> >> > >> Dear Tetsuo
> >> > >>
> >> > >> I think it is an important contribution to add vtu support, 
> >> > >> especially if it is binary/compressed, just ascii is not very useful. 
> >> > >> However we might need to discuss a bit on how to do it. As far as I 
> >> > >> can see you have used boost for xml writing. I think we had dropped 
> >> > >> our dependency on boost and I am not very keen on reintroducing a 
> >> > >> dependency on boost.
> >> > >>
> >> > >> Before we merge this, I would like to hear some arguments for one 
> >> > >> solution or another. The first thing to check is what others do. How 
> >> > >> is vtu export implemented in other software like e.g. fenics? What is 
> >> > >> the more future-proof way of implementing vtu support? What is a 
> >> > >> solution with least dependencies? If we have to depend on an external 
> >> > >> library it might be better to depend on vtk directly 
> >> > >> https://www.paraview.org/Wiki/VTK/Examples/Cxx/IO/WriteVTU
> >> > >>
> >> > >> Have you done some research regarding these questions?
> >> > >>
> >> > >> There is also another thing that I would like to ask you about. Could 
> >> > >> you please don't use markup in your git commit description? It might 
> >> > >> look nice in your git client but it looks ugly and difficult to read 
> >> > >> on other's systems.
> >> > >>
> >> > >> Best regards
> >> > >> Kostas
> >> > >>
> >> > >>
> >> > >>
> >> > >>
> >> > >> On Thu, May 7, 2020 at 2:07 AM Tetsuo Koyama <address@hidden> wrote:
> >> > >>>
> >> > >>> Dear getfem project
> >> > >>>
> >> > >>> Could you merge devel-tetsuo-xml?
> >> > >>> This branch is addition of vtu_export class.
> >> > >>> By using this class we can export xml unstructured grid format vtk
> >> > >>> (only ascii format and write_point_data).
> >> > >>> I tested it by using meshio package 
> >> > >>> (https://github.com/nschloe/meshio).
> >> > >>> In the future, the binary format and write_cell_data method may be 
> >> > >>> extended.
> >> > >>>
> >> > >>> Thank you for reading.
> >> > >>>
> >> > >>> BR Tetsuo
> >> > >>>