by the way, the "remove_dof_unused" function you should make it work
in-place on the passed by reference V vector, as was the code that you
replaced with this function. The line
gmm::copy(remove_dof_unused(V, pmf_dof_used, Q), W)
and the temporary W vector are completely unnecessary, and just add
redundant copy operations.

Best regards
Kostas

On Sun, May 10, 2020 at 10:16 PM Konstantinos Poulios <
logar...@googlemail.com> wrote:

> 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 <tkoyama...@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 <tkoyama...@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 <logar...@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 <
>> logar...@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 <tkoyama...@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