Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement insert points #69

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

sampotter
Copy link

I made a few small changes to implement TetGen's "constrained points" feature (addressing #60 and #68).

I'm not up on the internals of TetGen, so can't be completely confident that this works, but it seems to work. I added a simple example testing the feature under examples.

Running test_insert_points.py under IPython, I get:

In [28]: %run test_insert_points.py

In [29]: np.array(mesh.points)
Out[29]:
array([[0.        , 0.        , 0.        ],
       [0.        , 0.        , 1.        ],
       [1.        , 0.        , 0.        ],
       [1.        , 0.        , 1.        ],
       [1.        , 1.        , 0.        ],
       [1.        , 1.        , 1.        ],
       [0.        , 1.        , 0.        ],
       [0.        , 1.        , 1.        ],
       [0.33      , 0.7       , 0.91      ],
       [0.        , 0.        , 0.5       ],
       [0.5       , 0.        , 1.        ],
       [1.        , 0.        , 0.5       ],
       [0.5       , 0.        , 0.        ],
       [1.        , 0.5       , 1.        ],
       [1.        , 1.        , 0.5       ],
       [1.        , 0.5       , 0.        ],
       [0.5       , 1.        , 1.        ],
       [0.        , 1.        , 0.5       ],
       [0.5       , 1.        , 0.        ],
       [0.        , 0.5       , 1.        ],
       [0.        , 0.5       , 0.        ],
       [0.25      , 0.75      , 1.        ],
       [0.25      , 1.        , 0.75      ],
       [0.5625    , 0.4375    , 1.        ],
       [0.53125   , 0.71875   , 1.        ],
       [0.28125   , 0.46875   , 1.        ],
       [0.378125  , 0.621875  , 1.        ],
       [0.39414063, 0.76601562, 1.        ],
       [0.32847656, 0.70035156, 1.        ],
       [0.5       , 0.5       , 0.        ],
       [0.5625    , 1.        , 0.4375    ],
       [1.        , 0.5       , 0.5       ],
       [0.42539062, 0.48476562, 0.76393229],
       [0.        , 0.5       , 0.5       ],
       [0.53125   , 1.        , 0.71875   ],
       [0.        , 0.75      , 0.75      ]])

In [30]: np.array(mesh.elements)
Out[30]:
array([[ 6, 29, 18, 17],
       [ 8, 24, 26, 32],
       [29,  9, 32, 33],
       [29,  9, 12, 32],
       [11, 10, 23, 32],
       [25,  8, 26, 32],
       [ 6, 33, 29, 17],
       [ 0, 29,  9, 12],
       [ 2, 29, 12, 11],
       [31, 29, 11, 32],
       [ 5, 31, 24, 34],
       [21,  8, 27, 28],
       [22, 17, 30, 33],
       [ 5, 31, 13, 24],
       [31, 30, 32, 34],
       [ 4, 31, 30, 15],
       [17, 29, 30, 33],
       [19, 32, 33, 35],
       [ 0, 33, 29, 20],
       [ 2, 31, 29, 11],
       [ 8, 26, 27, 28],
       [30, 29, 31, 32],
       [25,  8, 32, 35],
       [ 8, 22, 32, 35],
       [19,  8, 21, 25],
       [32, 19,  9, 25],
       [ 8, 19, 21, 35],
       [ 0, 33,  9, 29],
       [24,  8, 27, 34],
       [29, 18, 17, 30],
       [22,  8, 32, 34],
       [ 8, 24, 32, 34],
       [ 7, 16, 21, 22],
       [19,  7, 21, 35],
       [ 7, 17, 22, 35],
       [19, 25, 32, 35],
       [19,  8, 25, 35],
       [30, 29, 32, 33],
       [13, 32, 11, 23],
       [10,  3, 11, 23],
       [29, 15, 30, 31],
       [22, 30, 32, 33],
       [19, 32,  9, 33],
       [24,  8, 26, 27],
       [21,  8, 22, 27],
       [16, 21, 22, 27],
       [ 8, 21, 22, 35],
       [ 8, 21, 25, 28],
       [ 8, 25, 26, 28],
       [32, 22, 33, 35],
       [31,  5, 14, 34],
       [16, 24, 27, 34],
       [23, 11, 13,  3],
       [ 2, 31, 15, 29],
       [ 6, 33, 20, 29],
       [31, 14, 30, 34],
       [30, 22, 32, 34],
       [29,  4, 18, 30],
       [ 4, 31, 14, 30],
       [ 4, 29, 15, 30],
       [ 1,  9, 25, 32],
       [29, 12, 11, 32],
       [ 8, 22, 27, 34],
       [21,  7, 22, 35],
       [31, 13, 24, 32],
       [32, 13, 11, 31],
       [22, 16, 27, 34],
       [22, 17, 33, 35],
       [23, 25, 26, 32],
       [23, 10, 25, 32],
       [24, 23, 26, 32],
       [ 9, 11, 12, 32],
       [10,  1, 25, 32],
       [ 9,  1, 10, 32],
       [ 9, 10, 11, 32],
       [24, 31, 32, 34],
       [16,  5, 24, 34],
       [13, 23, 24, 32],
       [ 9, 25, 19,  1]])

Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! Just one small request, then this should be ready to go.

from meshpy.tet import MeshInfo, Options, build

if __name__ == '__main__':
logger = logging.getLogger('test_insert_points.py')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a version of this to the (pytest-driven) tests under test/, to make sure this doesn't get inadvertently broken?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Please also take care of the Flake8 linter failures.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Fixed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bump

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't you mention in #70 that this PR was no longer needed? Could you explain?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. But this appears to be the correct way to implement this and IMO it makes sense to cover tetgen's interface regardless of whether there are parts of it that are a little redundant. Some people looking for a wrapper library for tetgen may be familiar with one approach and not the other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants