Implement OASIS file writer by c1ydehhx · Pull Request #674 · solvcon/modmesh

c1ydehhx · 2026-02-04T12:12:49Z

In this PR, I added OASIS device. This device support writer only currently. This writer implement manhattan delta format by OASIS spec.

For example, if we expected +340 for N/S direction, it will be encoded with 0xD0 0x0A, and -340 will be 0xD2 0x0A.

I added some test to make sure it work properly, and I add some edge case to make sure it will not buggy when met edge case.

yungyuc

Points to address:

Write OASIS I/O code in C++. Name it modmesh::OasisDevice. Wrap it to Python for testing.
Manually verify the written file by loading it using a third-party viewer and paste the screenshot.

yungyuc · 2026-02-04T13:26:59Z

modmesh/plot/plane_layer.py

 # POSSIBILITY OF SUCH DAMAGE.


+class ManhattanDeltaCodec:


Write I/O code in C++. Python is too slow to be taken seriously for I/O.

Name it modmesh::OasisDevice for now.

yungyuc · 2026-02-04T13:27:53Z

tests/test_plane_layer.py

 from modmesh.plot import plane_layer


+class ManhattanDeltaCodecTC(unittest.TestCase):


Unit tests for the OASIS I/O should be in Python.

c1ydehhx · 2026-02-05T16:03:25Z

Hi @yungyuc,
So far away, I already migrate Manhatten 1-delta writer (Named it OasisWriter) from Python to C++, and use pybind11 to wrap it as a module.

Write OASIS I/O code in C++. Name it modmesh::OasisDevice. Wrap it to Python for testing.

Done.

Manually verify the written file by loading it using a third-party viewer and paste the screenshot.

I'm concern that it will have too much diff in this PR if we want to output OASIS file. Althought we have geometry record now, we don't have some necessary OASIS header like magic bytes, START and END.

I would like to confirm that this PR should support OASIS file output or we can just test Manhatten 1-delta work properly with unit tests?

yungyuc · 2026-02-06T04:46:02Z

Linter did not pass. Please fix code format and then request for review again.

yungyuc

Some very basic points to address:

Do not make an intrusive alias in global namespace and for STL things like std::vector<uint8)_t>.
Rule of 5 for OasisDevice.
Name as OasisDevice::write, not writer, which is not a verb.

yungyuc · 2026-02-06T05:18:33Z

cpp/modmesh/oasis/oasis_device.hpp

+#include <cstdint>
+#include <vector>
+
+using bytes = std::vector<uint8_t>;


Do not make an intrusive alias like this. It is horrible to maintain.

There is nothing wrong with using std::vector<uint8)_t>? Just spell out the type.

It is horrible to add an alias in the global name space. Unacceptable almost always.

yungyuc · 2026-02-06T05:20:08Z

cpp/modmesh/oasis/oasis_device.hpp

+    static void append_1_delta(bytes & segment, int value);
+
+public:
+    OasisDevice() = default;


yungyuc · 2026-02-06T05:20:42Z

cpp/modmesh/oasis/oasis_device.hpp

+
+public:
+    OasisDevice() = default;
+    static bytes writer(std::vector<int32_t> & values);


Name it write, not writer, which is not a verb.

yungyuc · 2026-02-06T05:21:57Z

@tigercosmos @KHLee529 please help review

tigercosmos · 2026-02-06T16:09:15Z

cpp/modmesh/oasis/pymod/wrap_oasis.cpp

+namespace python
+{
+
+using namespace modmesh::oasis; // NOLINT(google-build-using-namespace)


I recommend remove this. Namespace is important for reading the code.

Good catch. modmesh::oasis namespace is not necessary and should be removed.

And, never using namespace unless it is in a very limited scope and can resolve extreme pain.

tigercosmos · 2026-02-06T16:09:52Z

cpp/modmesh/oasis/oasis_device.cpp

+
+void OasisDevice::append_1_delta(std::vector<uint8_t> & segment, int value)
+{
+    int dir_bit = value < 0 ? 1 : 0;


Please add const for all the places if applicable.

tigercosmos · 2026-02-06T16:10:50Z

cpp/modmesh/oasis/oasis_device.hpp

+namespace oasis
+{
+
+class OasisDevice


Please add document comments for classes and functions.

tigercosmos · 2026-02-06T16:12:21Z

cpp/modmesh/oasis/oasis_device.cpp

+    }
+}
+
+std::vector<uint8_t> OasisDevice::write(std::vector<int32_t> & values)


const std::vector<int32_t> & values

tigercosmos · 2026-02-06T16:12:43Z

cpp/modmesh/oasis/oasis_device.cpp

+namespace oasis
+{
+
+void OasisDevice::append_1_delta(std::vector<uint8_t> & segment, int value)


This can be a local function in cpp only.

tigercosmos · 2026-02-06T16:13:35Z

cpp/modmesh/oasis/oasis_device.cpp

+    while (payload >= 1)
+    {
+        int first_bit = payload >= 128 ? 1 : 0;
+        segment.push_back((first_bit << 7) | (payload % 128));


Please add comments to explain the algorithm. Including source doucment is even better.

tigercosmos · 2026-02-06T16:14:22Z

modmesh/plot/plane_layer.py

 # POSSIBILITY OF SUCH DAMAGE.


+class ManhattanDeltaCodec:


Please remove it.

tigercosmos · 2026-02-06T16:15:07Z

tests/test_oasis_device.py

+        device = modmesh.OasisDevice()
+        seg = device.writer([+340, +200, -340, -40, +300, -120, -300])
+
+        self.assertEqual(seg, [0xD0, 0x0A, 0xA0, 0x06, 0xD2,


It will be nice if you can explain what you try to test in the comments.

Use code comments to explain.

yungyuc

Points to address:

Do not use namespace modmesh::oasis.
Add very concise code comments for the key points in unit tests.

yungyuc · 2026-02-07T00:44:35Z

cpp/modmesh/oasis/pymod/wrap_oasis.cpp

+namespace python
+{
+
+using namespace modmesh::oasis; // NOLINT(google-build-using-namespace)


Good catch. modmesh::oasis namespace is not necessary and should be removed.

And, never using namespace unless it is in a very limited scope and can resolve extreme pain.

yungyuc · 2026-02-07T00:45:04Z

cpp/modmesh/oasis/oasis_device.hpp

+namespace modmesh
+{
+
+namespace oasis


Do not use namespace modmesh::oasis.

yungyuc · 2026-02-07T01:25:35Z

tests/test_oasis_device.py

+        device = modmesh.OasisDevice()
+        seg = device.writer([+340, +200, -340, -40, +300, -120, -300])
+
+        self.assertEqual(seg, [0xD0, 0x0A, 0xA0, 0x06, 0xD2,


Use code comments to explain.

yungyuc requested changes Feb 4, 2026

View reviewed changes

yungyuc assigned c1ydehhx Feb 4, 2026

yungyuc added the geometry Geometry entities and manipulation label Feb 4, 2026

yungyuc changed the title ~~Implement Manhattan Delta Codec writer~~ Implement OASIS file writer Feb 4, 2026

c1ydehhx force-pushed the manhattan-1-delta-writer branch from 8a2d55c to eb97718 Compare February 5, 2026 15:51

yungyuc requested changes Feb 6, 2026

View reviewed changes

yungyuc requested review from KHLee529 and tigercosmos February 6, 2026 05:21

c1ydehhx force-pushed the manhattan-1-delta-writer branch from 71b53ff to d3e62a3 Compare February 6, 2026 15:07

tigercosmos reviewed Feb 6, 2026

View reviewed changes

yungyuc requested changes Feb 7, 2026

View reviewed changes

Add OASIS Device (Only writer)

5607f69

c1ydehhx force-pushed the manhattan-1-delta-writer branch from 012b646 to 5607f69 Compare February 7, 2026 03:25

c1ydehhx added 3 commits February 7, 2026 11:40

Add comment on tests

50fdeef

Adjust function name in tests

7f576b3

Fix flake8

5db1eb2

		from modmesh.plot import plane_layer


		class ManhattanDeltaCodecTC(unittest.TestCase):

Conversation

c1ydehhx commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yungyuc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

c1ydehhx commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yungyuc commented Feb 6, 2026

Uh oh!

yungyuc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yungyuc commented Feb 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tigercosmos Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yungyuc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

c1ydehhx commented Feb 4, 2026 •

edited

Loading

yungyuc left a comment •

edited

Loading

c1ydehhx commented Feb 5, 2026 •

edited

Loading

yungyuc left a comment •

edited

Loading

tigercosmos Feb 6, 2026 •

edited

Loading