Conversation
| @@ -0,0 +1,291 @@ | |||
| We're working on adding support for physicsnemo Mesh objects to datapipes. this needs to be supported in a couple ways. | |||
There was a problem hiding this comment.
File looks accidentally committed - that's fine for a WIP branch, just marking this so we don't forget before merge.
| f" points={tuple(data.points.shape)}, " | ||
| f"cells={tuple(data.cells.shape)}, " | ||
| f"point_data={list(data.point_data.keys())}, " | ||
| f"cell_data={list(data.cell_data.keys())}" |
There was a problem hiding this comment.
If we commit this file (which we might not, it's a benchmark? But up to you), let's use the Mesh __repr__ here to deduplicate and/or reduce future compatibility risks
coreyjadams
force-pushed
the
pnm_mesh_datapipes
branch
from
0b2d336 to
132d72a
Compare
…face and volumetric meshes directly from physicsnemo mesh files.
|
|
||
|
|
||
| def benchmark_pipeline(name: str, dataloader: DataLoader, max_samples: int) -> None: | ||
| """Benchmark a single DataLoader pipeline.""" |
There was a problem hiding this comment.
Might want to write more details about the expected format of dataloader here in this docstring - what dataset objects are expected?
|
|
||
| # Default extension for physicsnemo mesh format (tensordict/tensorclass layout). | ||
| # Do not hardcode elsewhere so format can evolve. | ||
| DEFAULT_MESH_EXTENSION = ".pt" |
There was a problem hiding this comment.
Assuming we're doing memmap (in which case, we save to a directory), maybe we want to do ".mesh" or ".pnmesh" or something? Up to you. ".pt" is ok, but it generally indicates that one could torch.load("myfile.pt") which I don't think is possible with a memmap-saved TensorDict (but I could be wrong here)
| return {"source_path": str(self._paths[index])} | ||
|
|
||
| def __len__(self) -> int: | ||
| return self._length |
There was a problem hiding this comment.
Why precompute this? I'd recommend len(_paths) and removing the _length computation in __init__, just to reduce indirection and duplicate sources-of-truth (not that those can change - but this helps readability)
with multiple mesh based datapipes
2 participants
PhysicsNeMo Pull Request
Really early draft still, just for brainstorming and convergence with #1504 .
Description
Checklist
Dependencies
Review Process
All PRs are reviewed by the PhysicsNeMo team before merging.
Depending on which files are changed, GitHub may automatically assign a maintainer for review.
We are also testing AI-based code review tools (e.g., Greptile), which may add automated comments with a confidence score.
This score reflects the AI’s assessment of merge readiness and is not a qualitative judgment of your work, nor is
it an indication that the PR will be accepted / rejected.
AI-generated feedback should be reviewed critically for usefulness.
You are not required to respond to every AI comment, but they are intended to help both authors and reviewers.
Please react to Greptile comments with 👍 or 👎 to provide feedback on their accuracy.