nibabel icon indicating copy to clipboard operation
nibabel copied to clipboard

ENH: Reading and writing JSON/binary JSON based surface data (.jmsh and .bmsh)

Open fangq opened this issue 2 years ago • 6 comments

hi @effigies, I am following up on the discussions from #1065, and would like to use this thread to propose a patch to support .jmsh/.bmsh based surface mesh data formats.

To recap on my previous presentation during the surface mesh format meeting:

  • .jmsh is basically a plain JSON file using JMesh annotations to store surface mesh data structures (nodes as MeshVertex3, triangles as MeshTri3)
  • .bmsh is the same as .jmsh except that the underlying serialization uses a binary JSON format - BJData
  • via the JData specification (data-structure level annotations), both formats support data-record-level compression (zlib, gzip, 'lzma' and potentially others)

thanks to @neurolabusc's mesh format benchmarks, the loading speed and file sizes of these files are summarized/compared in this post.

Here, I would like to show you a lightweight patch (draft) to allow nibabel to read/write .jmsh and .bmsh files. A screenshot showing the interface can be found below.

This patch requires two external lightweight packages as dependencies, both can be installed via pip:

because I am not entirely familiar with nibabel's IO classes, currently, I based the JMesh class over FileBasedImage. I understand that CoordinateImage is currently being developed in #1090, and perhaps it is the better root class for adding new mesh formats.

Let me know if you have any comments about this draft. I would be happy to add some tests and open to suggestions to continue polishing this patch.

jmesh_nibabel

fangq avatar Jun 09 '22 04:06 fangq

Hello @fangq, thank you for submitting the Pull Request!

Line 13:19: E231 missing whitespace after ',' Line 13:26: E231 missing whitespace after ',' Line 13:34: E231 missing whitespace after ',' Line 20:19: E231 missing whitespace after ':' Line 21:14: E231 missing whitespace after ':' Line 22:23: E231 missing whitespace after ':' Line 23:13: E231 missing whitespace after ':' Line 24:17: E231 missing whitespace after ':' Line 28:17: E231 missing whitespace after ':' Line 32:21: E231 missing whitespace after ':' Line 33:14: E231 missing whitespace after ':' Line 34:12: E231 missing whitespace after ':' Line 38:1: E302 expected 2 blank lines, found 1 Line 74:12: E714 test for object identity should be 'is not' Line 77:12: E714 test for object identity should be 'is not' Line 78:86: E202 whitespace before '}' Line 85:12: E714 test for object identity should be 'is not' Line 86:83: E202 whitespace before '}' Line 102:1: E302 expected 2 blank lines, found 1 Line 119:29: E231 missing whitespace after ',' Line 124:5: E265 block comment should start with '# ' Line 126:5: E265 block comment should start with '# ' Line 130:5: E265 block comment should start with '# ' Line 132:5: E265 block comment should start with '# ' Line 146:37: E231 missing whitespace after ',' Line 149:5: E265 block comment should start with '# ' Line 151:5: E265 block comment should start with '# ' Line 163:37: E231 missing whitespace after ',' Line 166:5: E265 block comment should start with '# ' Line 168:5: E265 block comment should start with '# ' Line 170:39: E714 test for object identity should be 'is not' Line 170:101: E501 line too long (105 > 100 characters) Line 173:39: E714 test for object identity should be 'is not' Line 173:101: E501 line too long (105 > 100 characters) Line 176:44: E714 test for object identity should be 'is not' Line 176:101: E501 line too long (120 > 100 characters) Line 179:44: E714 test for object identity should be 'is not' Line 179:101: E501 line too long (120 > 100 characters) Line 184:1: E302 expected 2 blank lines, found 1 Line 209:34: E714 test for object identity should be 'is not' Line 210:31: E225 missing whitespace around operator Line 211:34: E714 test for object identity should be 'is not' Line 212:43: E714 test for object identity should be 'is not' Line 213:36: E225 missing whitespace around operator Line 215:36: E225 missing whitespace around operator Line 217:34: E714 test for object identity should be 'is not' Line 218:38: E272 multiple spaces before keyword Line 218:44: E714 test for object identity should be 'is not' Line 219:33: E225 missing whitespace around operator Line 221:33: E225 missing whitespace around operator Line 225:1: E305 expected 2 blank lines after class or function definition, found 1

To test for issues locally, pip install flake8 and then run flake8 nibabel.

pep8speaks avatar Jun 09 '22 04:06 pep8speaks

Thanks @fangq! This is great to see. If I'm reading this right, jmsh/bmsh are specifically surface meshes, not data sampled to surface meshes? If so, then it corresponds to the TriangularMesh API from BIAP 9, not really the CoordinateImage itself.

One question I have is if it would be possible to load the metadata from files and then construct something like an array proxy so that the data arrays do not need to be loaded until called for? If I recall, your files are not actually gzipped, only the data arrays, so we can't use our existing file handlers and proxies as drop-in loaders, but I suspect we could figure something out.

effigies avatar Jun 09 '22 20:06 effigies

Thanks @fangq! This is great to see. If I'm reading this right, jmsh/bmsh are specifically surface meshes, not data sampled to surface meshes? If so, then it corresponds to the TriangularMesh API from BIAP 9, not really the CoordinateImage itself.

Thanks for the feedback. The JMesh spec actually permits one to attach node or triangle-associated data as Properties. The currently supported property keywords include Normal, Color, Value, Tag, and Size. However, for simplicity, in this PR, I only exposed the Tag properties as JMesh class's nodelabel and facelabel properties - others can also be easily exposed if see a need.

In this case, should I derive JMesh from TriangularMesh class?

One question I have is if it would be possible to load the metadata from files and then construct something like an array proxy so that the data arrays do not need to be loaded until called for?

Loading array buffers from the file is handled in the parser level (i.e. the external bjdata and jdata moduels), and it currently always loads the data to memory when parsing a file. When the data buffer is compressed, my understanding is that numpy.memmap or arrayproxy may not likely to work. But for raw/uncompressed bjdata (raw.bmsh) files, I could potentially add some code to allow the parser to return numpy.memmap objects if user prefers. For now, let's assume this memmap based access is not yet supported.

By the way, do you have a code formatter (something like this for C++) so that I can get rid of CI warnings?

also, what command to run all tests in nibabel? I just want to follow the examples of other modules and start building some tests.

fangq avatar Jun 10 '22 01:06 fangq