the other thing is the big overlapping between the vtk_export class and the new vtu_export class. Duplicating code is not a very good strategy. I think you should just keep the old vtk_export class and add an optional argument for the user to choose if exporting to xml format or not.
Best regards Kostas On Sun, May 10, 2020 at 10:23 PM Konstantinos Poulios < [email protected]> wrote: > 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 < > [email protected]> 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 <[email protected]> >> 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 <[email protected]>: >>> > >>> > 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 <[email protected]>: >>> > > >>> > > 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 < >>> [email protected]> 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 <[email protected]> >>> 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 >>> > >>> >>> >>
