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 >> > >>> >> >