Added support for loading native object for openvdb - #170
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #170 +/- ##
==========================================
+ Coverage 90.59% 90.76% +0.16%
==========================================
Files 6 6
Lines 946 985 +39
Branches 126 131 +5
==========================================
+ Hits 857 894 +37
- Misses 52 53 +1
- Partials 37 38 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Is the objective of the PR to enable #162 for openvdb objects only or implement it for all available formats? It's totally ok to only do it for OpenVDB, I just want to know when reviewing the PR. |
orbeckst
left a comment
orbeckst
left a comment
There was a problem hiding this comment.
Great that you're starting the work on #162 . The type-selection mechanism needs to be reworked.
We also need to have a more general architecture for Grid for (1) detecting the format, (2) dispatching to the correct loader. We can decide to do this in a separate PR if for right now we're only focusing on VDB.
| for key in self.vdb_grid.metadata: | ||
| try: | ||
| self.metadata[key] = self.vdb_grid[key] | ||
| except (TypeError, ValueError): |
There was a problem hiding this comment.
Under which conditions does that fail? I am always suspicious of try/except that passes.
There was a problem hiding this comment.
vec3f can be one of the example. But I can create a path for a warning to be raised for this
| self.origin = v_origin | ||
| self.delta = v_delta | ||
|
|
||
| dtype = np.dtype("float32") |
There was a problem hiding this comment.
I think it's cleaner to set the dtype to the default in an else block of the for loop:
for numpy_dtype, vdb_names in self._DATATYPES.items():
name_dtype = vdb_names[0]
...
if vdb_class_name == canonical_name:
dtype = numpy_dtype
break
else:
# could not find a matching dtype, use default
dtype = np.float32Or maybe should we use float64? @BradyAJohnston @PardhavMaradani do you have an opinion?
There was a problem hiding this comment.
You should also log a warning if you encounter an unknown type and use the default.
| shape = tuple(np.array(bbox[1]) - np.array(bbox[0]) + 1) | ||
|
|
||
| self.grid = np.zeros(shape, dtype=dtype) | ||
| print(dtype) |
| world = native_grid.transform.indexToWorld((0, 0, 0)) | ||
| assert_allclose([world[0], world[1], world[2]], g.origin, rtol=1e-5) | ||
|
|
||
| def test_extract_from_vdb_grid(self, grid345): |
There was a problem hiding this comment.
parameterize the test to try out all normally supported vdb gridtypes
|
For right now I linked this PR to the OpenVDB-only issue #171 |
Co-authored-by: Oliver Beckstein <orbeckst@gmail.com>
Co-authored-by: Oliver Beckstein <orbeckst@gmail.com>
Documentation build overview
11 files changed ·
|
- updated init signature of OpenDX.DXClass to include name and components - consistently use super() in DXClass-child classes - use zero-arg super() calls
fixed initialization of DXClasses
|
Hi Prof @orbeckst |
orbeckst
left a comment
orbeckst
left a comment
There was a problem hiding this comment.
Thanks for making the last changes. This looks ready, nice work!
Apologies for the delayed review.
Thanks professor @orbeckst . Learned a lot about openvdb and the other APIs during this!! 😊 |
2 participants
Fixes #171
Changes -
_extract_from_vdb_gridthat gets the delta, origin, data type and metadata, if a native object is passed - OpenVDB.py_extract_from_vdb_grid- test_vdb.py@orbeckst please have a look at this when you are free and have time.
Also, made a new section in changelog, with enhancement in it