Fix type comparison - #67
Merged
Merged
Conversation
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Python |  |
Jul 7, 2026 4:36p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
Owner
|
The tests are failing and I think it is because of older versions of torchfem. For compatibility reasons we could keep both checks like this etype = Solid.etype
if etype is Tetra1 or isinstance(etype, Tetra1):
cell_types = [pyvista.CellType.TETRA] * Solid.n_elem
elif etype is Tetra2 or isinstance(etype, Tetra2):
cell_types = [pyvista.CellType.QUADRATIC_TETRA] * Solid.n_elem
elif etype is Hexa1 or isinstance(etype, Hexa1):
cell_types = [pyvista.CellType.HEXAHEDRON] * Solid.n_elem
elif etype is Hexa2 or isinstance(etype, Hexa2):
cell_types = [pyvista.CellType.QUADRATIC_HEXAHEDRON] * Solid.n_elem
else:
raise TypeError(f"Unsupported element type: {etype} ({type(etype)})") |
Owner
|
looks good to me, thanks! |
mkofler96
added a commit
that referenced
this pull request
Jul 14, 2026
* fix bounds slicing for xmin and xmax calculations (#64) * Add homogenization (#65) * Add test experiment for homogenization model with training artifacts - Introduced new experiment directory for testing homogenization. - Added latest model, optimizer parameters, latent codes, and logs as binary files. - Created specs.json to define network architecture and training parameters. - Added training summary in training_summary.json. - Implemented a test function to train the homogenization model in test_train_model.py. * Add homogenization network and update training loss calculation * added xlim option back to plot sdf * Update test experiment specs and data directory for homogenization model * Refactor homogenization model tests to use shared data directory fixture * set default dtype to float32 in test train model * reduced test duration by reducing sampling points * fix retry 429 error * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix negative Jacobian at mesh resolution 10 by filtering tets in float64 The orientation check ran in float32 but torchfem evaluates Jacobians in float64 (after verts.to(float64)). Tets with a tiny positive float32 volume can flip negative in float64, causing the "Negative Jacobian" exception. Compute the final validity mask in float64 so the filter is consistent with torchfem. * added HF secret --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> * Fix wrong module loading (#66) * Refactor model loading to handle module prefix in state_dict more efficiently * added some additional output checks for the model export * replaced deprecated torch.linalg.norm argument axes * Fix type comparison in optimization (#67) * Fix type comparison * Add previous type checks for compatibility reasons --------- Co-authored-by: Aron Längert <e11916873@student.ilsb.tuwien.ac.at> * added pypi publish workflow * Fix publish workflow (#69) * added pypi environment * fixed version criteria * added guard to only trigger on main * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Skip publish job on non-tag pushes in PyPI workflow (#70) * Initial plan * chore: start publish failure investigation Co-authored-by: mkofler96 <18218171+mkofler96@users.noreply.github.com> * fix(ci): skip publish job on branch pushes Co-authored-by: mkofler96 <18218171+mkofler96@users.noreply.github.com> * chore: revert unintended uv lockfile changes Co-authored-by: mkofler96 <18218171+mkofler96@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mkofler96 <18218171+mkofler96@users.noreply.github.com> * add threshold_factor argument * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Michael Kofler <michael.kofler@tuwien.ac.at> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Aron Längert <e11916873@student.ilsb.tuwien.ac.at> Co-authored-by: Michael Kofler <rasta.kof@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: mkofler96 <18218171+mkofler96@users.noreply.github.com>
mkofler96
added a commit
that referenced
this pull request
Jul 15, 2026
* Fix inverted tetrahedra in FlexiCubes volume mesh output FlexiCubes' _tetrahedralize builds tets from two sub-procedures (surface pyramids and interior edges) whose vertex orderings do not share a consistent winding. As a result a large fraction of elements came out inverted (negative signed volume) - nearly all surface tets and ~40% of interior tets - which breaks FEA solvers that require a positive signed volume / Jacobian on every element. Add _orient_tets to normalize every tet to positive orientation by swapping two vertices where the signed volume is negative. This only reorders integer indices, so element geometry, |volume|, and gradients to the vertices are all preserved (the extractor stays differentiable). Add regression tests asserting no inverted tets are produced. * Remove degenerate zero-volume tets from FlexiCubes output The interior tetrahedralization sub-procedure can emit elements whose four vertices are exactly coplanar (the two dual-mesh vertices land symmetric about the grid edge), yielding zero-volume tets that fail the positive-Jacobian requirement of FEA solvers just like inverted ones. Drop these elements in _orient_tets and log 'removed x elements with 0 volume'. Coplanarity is detected with a tolerance relative to the Hadamard bound of the determinant rather than an exact zero compare, since the rounding of an exactly-degenerate triple product depends on association order. The measured relative volumes are cleanly bimodal (degenerates at <=1e-7, real elements at >=1e-5), so the 1e-5 cutoff removes only degenerate elements. * Drop caplog assertion from degenerate-tet test * style: format code with Black This commit fixes the style issues introduced in efe750d according to the output from Black. Details: #63 * Address review: explicit grid bounds in test, no_grad in _orient_tets construct_voxel_grid defaults to bounds [-0.05, 1.05] in this repo, so scaling by 2 did not produce the [-1, 1] domain the test comment claimed; pass explicit bounds instead. Also wrap the orientation classification in torch.no_grad() since it only derives integer index masks. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add the threshold factor as an argument and merge main (#71) * fix bounds slicing for xmin and xmax calculations (#64) * Add homogenization (#65) * Add test experiment for homogenization model with training artifacts - Introduced new experiment directory for testing homogenization. - Added latest model, optimizer parameters, latent codes, and logs as binary files. - Created specs.json to define network architecture and training parameters. - Added training summary in training_summary.json. - Implemented a test function to train the homogenization model in test_train_model.py. * Add homogenization network and update training loss calculation * added xlim option back to plot sdf * Update test experiment specs and data directory for homogenization model * Refactor homogenization model tests to use shared data directory fixture * set default dtype to float32 in test train model * reduced test duration by reducing sampling points * fix retry 429 error * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix negative Jacobian at mesh resolution 10 by filtering tets in float64 The orientation check ran in float32 but torchfem evaluates Jacobians in float64 (after verts.to(float64)). Tets with a tiny positive float32 volume can flip negative in float64, causing the "Negative Jacobian" exception. Compute the final validity mask in float64 so the filter is consistent with torchfem. * added HF secret --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> * Fix wrong module loading (#66) * Refactor model loading to handle module prefix in state_dict more efficiently * added some additional output checks for the model export * replaced deprecated torch.linalg.norm argument axes * Fix type comparison in optimization (#67) * Fix type comparison * Add previous type checks for compatibility reasons --------- Co-authored-by: Aron Längert <e11916873@student.ilsb.tuwien.ac.at> * added pypi publish workflow * Fix publish workflow (#69) * added pypi environment * fixed version criteria * added guard to only trigger on main * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Skip publish job on non-tag pushes in PyPI workflow (#70) * Initial plan * chore: start publish failure investigation Co-authored-by: mkofler96 <18218171+mkofler96@users.noreply.github.com> * fix(ci): skip publish job on branch pushes Co-authored-by: mkofler96 <18218171+mkofler96@users.noreply.github.com> * chore: revert unintended uv lockfile changes Co-authored-by: mkofler96 <18218171+mkofler96@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mkofler96 <18218171+mkofler96@users.noreply.github.com> * add threshold_factor argument * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Michael Kofler <michael.kofler@tuwien.ac.at> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Aron Längert <e11916873@student.ilsb.tuwien.ac.at> Co-authored-by: Michael Kofler <rasta.kof@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: mkofler96 <18218171+mkofler96@users.noreply.github.com> * Revert "add the threshold factor as an argument and merge main (#71)" This reverts commit 7081e77. * add threshold_factor argument * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Aron Längert <18172321+ALaengert99@users.noreply.github.com> Co-authored-by: Aron Längert <e11916873@student.ilsb.tuwien.ac.at> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: mkofler96 <18218171+mkofler96@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix type comparison as Solid.etype is an instance of the respective class