getfem-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Tetsuo Koyama
Subject: Re: [Getfem-commits] please merge branch devel-tetsuo-xml
Date: Sun, 4 Oct 2020 10:02:09 +0900

Dear Yves and Kostas

I hope you are well.  I finally completed adding the VTU binary output and its testing.

I added "tests/python/check_export_vtu.py" to test the output of the VTK/VTU file in more detail.
To run this test you need to use pyvisa which can be installed by command.
$ pip3 install pyvista

Could you please merge devel-tetsuo-xml-binary-squash ?

Best Regards

2020年5月28日(木) 20:54 Tetsuo Koyama <tkoyama010@gmail.com>:
Dear Yves

Thank you for your merge.

Dear Kostas

Thank you for your comment. I will fix it in next branch.

Best regards Tetsuo

2020年5月28日(木) 19:14 Yves Renard <yves.renard@insa-lyon.fr>:
>
> Dear all,
>
> I did the merge.
>
> Best regards,
>
> Yves
>
> ----- Mail original -----
> De: "logari81" <logari81@googlemail.com>
> À: "Tetsuo Koyama" <tkoyama010@gmail.com>
> Cc: "yves renard" <yves.renard@insa-lyon.fr>, "getfem-commits" <getfem-commits@nongnu.org>
> Envoyé: Mercredi 27 Mai 2020 16:47:40
> Objet: Re: [Getfem-commits] please merge branch devel-tetsuo-xml
>
> it looks good and clean. Thanks. I will do the merge later if Yves doesn't
> do it first.
>
> Just a minor comment. Now in modern C++ we use to rewrite the old loops
> like this
>
> for (size_type i=0; i < s.size(); ++i) {
>   write_val(int(vtk_simplex_code[s[i].dim()]));
> }
>
> to
>
> for (const auto &val : s)
>   write_val(int(vtk_simplex_code[val.dim()]));
>
> if possible. You can also drop the extra brackets {} if they are not
> necessary.
>
> BR
> Kostas
>
> On Wed, May 27, 2020 at 3:03 AM Tetsuo Koyama <tkoyama010@gmail.com> wrote:
>
> > P.S. branch is devel-tetsuo-xml-slices
> >
> > 2020年5月27日(水) 10:02 Tetsuo Koyama <tkoyama010@gmail.com>:
> > >
> > > Dear all
> > >
> > > I added the following functions and confirmed that the test passed. It
> > > is ready for merge.
> > > > 2) make exporting slices work for VTU. Based on my refactored version,
> > it shouldn't be difficult.
> > >
> > > I plan to add 1) 3) after this.
> > >
> > > BR Tetsuo
> > >
> > > 2020年5月26日(火) 2:01 Yves Renard <yves.renard@insa-lyon.fr>:
> > > >
> > > >
> > > > Dear all,
> > > >
> > > > Ok, I merged the branch and I will proceed with 5.4.1 patch version.
> > > >
> > > > Best regards,
> > > >
> > > > Yves
> > > >
> > > > ----- Mail original -----
> > > > De: "logari81" <logari81@googlemail.com>
> > > > À: "Tetsuo Koyama" <tkoyama010@gmail.com>
> > > > Cc: "getfem-commits" <getfem-commits@nongnu.org>, "yves renard" <
> > yves.renard@insa-lyon.fr>
> > > > Envoyé: Lundi 25 Mai 2020 15:04:23
> > > > Objet: Re: [Getfem-commits] please merge branch devel-tetsuo-xml
> > > >
> > > > Dear Tetsuo,
> > > >
> > > > Great, thanks for testing and for the original vtu implementation. It
> > is a
> > > > very useful feature.
> > > >
> > > > I will let Yves do the merge. I think he will prepare a 5.4.1 version
> > soon
> > > > to fix the issues with 5.4. on Ubuntu 20.04.
> > > >
> > > > Best regards
> > > > Kostas
> > > >
> > > >
> > > > On Mon, May 25, 2020 at 2:32 PM Tetsuo Koyama <tkoyama010@gmail.com>
> > wrote:
> > > >
> > > > > Dear Kostas
> > > > >
> > > > > My test of branch devel-logari81-xml was passed.
> > > > > Your branch is awesome.
> > > > > I think it is a very good idea to add vtk as a option too.
> > > > > Could you merge this branch?
> > > > > After that I will checkout new branch to add more functionality.
> > > > >
> > > > > Best regards Tetsuo
> > > > >
> > > > > 2020年5月24日(日) 20:49 Tetsuo Koyama <tkoyama010@gmail.com>:
> > > > > >
> > > > > > 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 <logari81@googlemail.com>:
> > > > > > >
> > > > > > > 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 <
> > tkoyama010@gmail.com>
> > > > > 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 <
> > logari81@googlemail.com>:
> > > > > > >> > >
> > > > > > >> > > 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 <
> > > > > tkoyama010@gmail.com> 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 <tkoyama010@gmail.com>:
> > > > > > >> > >> >
> > > > > > >> > >> > 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 <
> > > > > logari81@googlemail.com>:
> > > > > > >> > >> > >
> > > > > > >> > >> > > 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 <
> > > > > logari81@googlemail.com> 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 <
> > > > > tkoyama010@gmail.com> 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
> > > > > > >> > >> > >>>
> > > > >
> >

reply via email to

[Prev in Thread] Current Thread [Next in Thread]