|
| From: | Tetsuo Koyama |
| Subject: | Re: [Getfem-commits] please merge branch devel-tetsuo-xml |
| Date: | Mon, 5 Oct 2020 13:43:32 +0900 |
Dear Kostas> I had misunderstood the test, obviously you cannot hardcode the result that needs to be checked. So you will just have to skip the test if pyvista is not available. But I still believe we should not require or check for pyvista in the configure script, just for one test case. So please add this check in the test itself.Got it. Thanks for your message.Best regards Tetsuo.2020年10月5日(月) 5:42 Konstantinos Poulios <logari81@googlemail.com>:Dear TetsuoI had misunderstood the test, obviously you cannot hardcode the result that needs to be checked. So you will just have to skip the test if pyvista is not available. But I still believe we should not require or check for pyvista in the configure script, just for one test case. So please add this check in the test itself.Best regardsKostasOn Sun, Oct 4, 2020 at 5:41 PM Tetsuo Koyama <tkoyama010@gmail.com> wrote:Dear Yves and KostasThank you for your feedback.I will follow the last option. I will contact you later.2020年10月4日(日) 23:39 Konstantinos Poulios <logari81@googlemail.com>:Dear TetsuoRegarding the rest of the code it looks nice and clean. No objections from my side. Thanks for the nice work.Best regardsKostasOn Sun, Oct 4, 2020 at 4:23 PM Konstantinos Poulios <logari81@googlemail.com> wrote:Hi,I think the last option is the simplest.@Tetsuo you could substitute the linesactual = unstructured_grid.points[:, 0]andactual = unstructured_grid.cell_connectivitywithactual = [.....] <--- hardcoded resultif unstructured_grid: <--- unstructured_grid==None if pyvista is missingactual = unstructured_grid.points[:, 0]andactual = [.....] <--- hardcoded resultif unstructured_grid: <--- unstructured_grid==None if pyvista is missingactual = unstructured_grid.cell_connectivityOtherwise thank for your code, I will also have a look at the rest.Best regardsKostasOn Sun, Oct 4, 2020 at 4:06 PM Yves Renard <yves.renard@insa-lyon.fr> wrote:
Dear Tetsuo and Kostas,
Tetsuo, thank you for your developpment.
I understand that you introduced a new dependance on the pyvista python package (only for a test program). There is several possibilities :
- Force the presence of the package in configure.ac (i.e. stop the configure if the package is not present)
- Detect the presence in a configure variable such as PYTHON_PYVISTA and disable the test if the package is not installed (eventually warning at the end of the configure file when it is nor present)
- Detect directly the presence of the package in the python test program.
What do you think is the best ?
Best regards,
Yves
----- Mail original -----
De: "Tetsuo Koyama" <tkoyama010@gmail.com>
À: "yves renard" <yves.renard@insa-lyon.fr>, "logari81" <logari81@googlemail.com>, "getfem-commits" <getfem-commits@nongnu.org>
Envoyé: Dimanche 4 Octobre 2020 03:46:02
Objet: Re: [Getfem-commits] please merge branch devel-tetsuo-xml
P.S. Sorry, I pushed devel-tetsuo-xml-binary by mistake. Delete
devel-tetsuo-xml-binary and merge devel-tetsuo-xml-binary-squash with the
commit squashed.
2020年10月4日(日) 10:02 Tetsuo Koyama <tkoyama010@gmail.com>:
> 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
>> > > > > > > >> > >> > >>>
>> > > > > >
>> > >
>>
>
| [Prev in Thread] | Current Thread | [Next in Thread] |