<div dir="ltr">Hi, Markus!<br><div><div class="gmail_extra"><br>I have addressed some of the changes listed below. I will finish the remaining ones in the next few days. <br><br></div><div class="gmail_extra">In the last few weeks I was also working the 3ed point in the plan Oliver made：<br>
<pre>3. Implement an HDF5-based file format for psurface objects
Nobody will be able to use your stuff as long as we rely on the AmiraMesh format.
(The format itself is not a secret, but the support library is difficult to come by).
- AmiraMesh is a general-purpose file format for all kinds of visualization-related
data. Read the file psurface/README to learn how psurface objects are stored in
AmiraMesh files.
- Familiarize yourself with HDF5 (<a href="http://www.hdfgroup.org/HDF5/">http://www.hdfgroup.org/HDF5/</a>) Find a way to
store a psurface object in HDF5 instead of AmiraMesh. Here it is okay to use
the HDF5 support library.
- Implement HDF5 reading and writing in psurface-convert</pre></div><div class="gmail_extra">After doing some research on hdf5, I find that both paraview and visit could not read unstructured hdf5 file directly. So I write some function that could write the xmdf file. xmdf file provide a description on the data structure while the heavy data are stored in hdf5. The paraviw could read hdf5 thought xmdf. <br>
<br></div><div class="gmail_extra">I have updated the hdf5 code (hdf5IO.cpp and hdf5test.cpp) and the new Makefile.am into my git directory in <a href="https://gitorious.org/gcos/psurface-convert" target="_blank">https://gitorious.org/gcos/psurface-convert. </a><br>
</div><div class="gmail_extra">I have it with the sphere object, the output hdf5 file psurface.h5 and xdmf file xdmf3d.xmf is also put to that directory. <br><br>I am sorry for the late reply. My grandma passed away last week, so I
changed the summer plan and travelled back to China last weekend. I spent some
time on visa application this week. There are good internet connection at home and I will be working on this project from home. <br> <br></div><div class="gmail_extra">Best,<br></div><div class="gmail_extra">Xiaoxue<br>
</div><div class="gmail_extra"><a href="https://gitorious.org/gcos/psurface-convert" target="_blank"></a></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Jul 27, 2013 at 5:08 AM, Markus Blatt <span dir="ltr"><<a href="mailto:markus@dr-blatt.de" target="_blank">markus@dr-blatt.de></a></span> wrote
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>On Fri, Jul 26, 2013 at 09:39:10PM -0400, Xiaoxue Gong wrote:<br>
> I have made a clean repository in<br>
> <a href="https://gitorious.org/gcos/psurface-convert" target="_blank">https://gitorious.org/gcos/psurface-convert</a> <br>
> Documentation on the format I used on storing psurface information into vtk<br>
> files and documentation on how to replicate my results is included in<br>
> ./doc/REARDME and ./doc/note<br>
<br>
</div>Thanks, now it looks much cleaner. Still I think there are a few<br>
things that could be changed:<br>
<br>
+ When I clone the the repository the directory tree looks like this<br>
psurface-convert/<br>
|<br>
-------- dune-surface-convert<br>
|<br>
---- dune<br>
|<br>
---- src<br>
...<br>
<br>
Personally I would have expected the files directly in the root<br>
directory, i.e. without the additional dune-psurface-convert<br>
directory in between.<br>
BTW: I think this is a minor issue, and can easily be resolved (even<br>
later).<br>
<br>
+ With your code being a dune module, I would have exspected it to<br>
adhere to our guidlines.<br>
- Use CamelCase for the method and class names. Method names start<br>
with a lowercase letter, class names with an uppercase letter.<br>
(e.g. VTKPWriter::IndexMapBack should be VTKPWriter::indexMapBack<br>
- The root namespace should probably be Dune (you could use a<br>
nested PSurface namespace inside it)<br></blockquote><div>I have changed the method and class names according to CamelCase.<br></div><div>The root name is changed to psurface<br><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ Your code is indented very inconsequently. This makes it very hard to read for others. It is probably due to using tabs. You should</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
not use them for indentation! There is the possibility to enforce<br>
not using tabs when commiting with dunecontrol. Please ask on this<br>
list how to use this with your repository.</blockquote><div><span name="Christoph Grüninger">Christoph have given me some instruction on this. I have not figure out the details yet. I will do this during this week. </span><br>
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Most probably you can<br>
tell your favorite editor not use tabs for indentation.<br>
<br>
+ You should make use of information hiding. Currently all the<br>
methods of VTKPWriter are declared public. You should limit public<br>
methods to those that should be exposed to user (e.g. for<br>
writing/reading the data). I can see why you did, as you seem to<br>
use all methods and members in writevtk. Maybe this should be a<br>
member functionod VTKPWriter, too?<br></blockquote><div>I have put the public function into the VtkPWriter. <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
+ Currently one can only write PSurface<2,float>. Is that intended?<br>
Shouldn't we be able to write others like PSurface<3,double>, too?<br></blockquote><div>It could work with PSurface<2,double><br></div><div>But I am not sure whether it could work with Psurface<3,ctype>. <br>
</div><div>The function to read am file could only deal with 2d (since it have Psurface<2,ctype> as <br>input. So I could only test with 2d case. What is the difference between 2d and 3d? <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
+ IMHO vtuIO.[hc]pp should be usable from other modules<br>
too. Therefore I would move to an appropriate subdirectory below<br>
dune/ (take a look at the core modules to see what might be<br>
appropriate). If you stick with the free method writevtk then<br>
vtuIO.cpp should be put into a library.<br>
<br>
+ Please try to document your code along (or even before) writing it<br>
using the doxygen syntax. Most developers (me, too) are not<br>
disciplined enough to document their code after writing it. In the<br>
rare occasion the we are and that there is time left, we are often<br>
having a hard time understanding it ourselves (which is a shame and<br>
waste of time). I would recommend documenting it properly _now_.<br>
<br>
+ What is up with the weired AC_ARG_WITH clauses in <a href="http://configure.ac" target="_blank">configure.ac</a>? I do not<br>
think that they are needed and doubt that they serve their intended<br>
purpose.<br></blockquote><div> I will deal with these issues during this week. <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Cheers,<br>
<div><div><br>
Markus<br>
--<br>
Do you need more support with DUNE or HPC in general?<br>
<br>
Dr. Markus Blatt - HPC-Simulation-Software & Services <a href="http://www.dr-blatt.de" target="_blank">http://www.dr-blatt.de</a><br>
Hans-Bunte-Str. 8-10, 69123 Heidelberg, Germany<br>
Tel.: +49 (0) 160 97590858 Fax: +49 (0)322 1108991658<br>
<br>
_______________________________________________<br>
Dune-devel mailing list<br>
<a href="mailto:Dune-devel@dune-project.org" target="_blank">Dune-devel@dune-project.org</a><br>
<a href="http://lists.dune-project.org/mailman/listinfo/dune-devel" target="_blank">http://lists.dune-project.org/mailman/listinfo/dune-devel</a><br>
</div></div></blockquote></div><br></div></div></div>