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

[Tetsuo Koyama]

Dear Kostas

Sorry for my late reply and thank you for your refactoring.
I'll check it and will continue you proposal. After I finished
development, I'll contact getfem project to merge.
Thanks for your advice.

Best regards Tetsuo

2020年5月24日(日) 4:13 Konstantinos Poulios <address@hidden>:
>
> Dear Tetsuo,
>
> I have revised your code and refactored it in my logari81-devel-xml branch. 
> Can you test that my refactored version works as your original version? I 
> removed your "only_mesh" option intentionally.
>
> Moreover, it would be nice if you could:
> 1) implement the scripting interface for vtu export.
> 2) make exporting slices work for VTU. Based on my refactored version, it 
> shouldn't be difficult.
> 3) implement the binary version of VTU.
>
> Best regards
> Kostas
>
> On Wed, May 13, 2020 at 3:35 PM Tetsuo Koyama <address@hidden> wrote:
>>
>> 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
>> > >> > >>>