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

Adding copy and verify to Curve and Surface constructors. #163

Merged
merged 2 commits into from
Jan 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/python/bezier/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class Base:
shape. Must be convertible to a 2D NumPy array of floating point
values, where the columns are the nodes and the rows correspond to
each dimension the shape occurs in.
_copy (bool): Flag indicating if the nodes should be copied before
copy (bool): Flag indicating if the nodes should be copied before
being stored. Defaults to :data:`True` since callers may
freely mutate ``nodes`` after passing in.

Expand All @@ -38,11 +38,11 @@ class Base:
__slots__ = ("_dimension", "_nodes")
_degree = -1

def __init__(self, nodes, _copy=True):
def __init__(self, nodes, copy=True):
nodes_np = sequence_to_array(nodes)
dimension, _ = nodes_np.shape
self._dimension = dimension
if _copy:
if copy:
self._nodes = nodes_np.copy(order="F")
else:
self._nodes = nodes_np
Expand Down
54 changes: 41 additions & 13 deletions src/python/bezier/curve.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,11 @@ class Curve(_base.Base):
degree (int): The degree of the curve. This is assumed to
correctly correspond to the number of ``nodes``. Use
:meth:`from_nodes` if the degree has not yet been computed.
_copy (bool): Flag indicating if the nodes should be copied before
copy (bool): Flag indicating if the nodes should be copied before
being stored. Defaults to :data:`True` since callers may
freely mutate ``nodes`` after passing in.
verify (bool): Flag indicating if the degree should be verified against
the number of nodes. Defaults to :data:`True`.
"""

__slots__ = (
Expand All @@ -95,12 +97,13 @@ class Curve(_base.Base):
"_degree", # From constructor
)

def __init__(self, nodes, degree, _copy=True):
super(Curve, self).__init__(nodes, _copy=_copy)
def __init__(self, nodes, degree, *, copy=True, verify=True):
super(Curve, self).__init__(nodes, copy=copy)
self._degree = degree
self._verify_degree(verify)

@classmethod
def from_nodes(cls, nodes, _copy=True):
def from_nodes(cls, nodes, copy=True):
"""Create a :class:`.Curve` from nodes.

Computes the ``degree`` based on the shape of ``nodes``.
Expand All @@ -110,7 +113,7 @@ def from_nodes(cls, nodes, _copy=True):
Must be convertible to a 2D NumPy array of floating point
values, where the columns represent each node while the rows
are the dimension of the ambient space.
_copy (bool): Flag indicating if the nodes should be copied before
copy (bool): Flag indicating if the nodes should be copied before
being stored. Defaults to :data:`True` since callers may
freely mutate ``nodes`` after passing in.

Expand All @@ -120,7 +123,7 @@ def from_nodes(cls, nodes, _copy=True):
nodes_np = _base.sequence_to_array(nodes)
_, num_nodes = nodes_np.shape
degree = cls._get_degree(num_nodes)
return cls(nodes_np, degree, _copy=_copy)
return cls(nodes_np, degree, copy=copy, verify=False)

@staticmethod
def _get_degree(num_nodes):
Expand All @@ -134,6 +137,31 @@ def _get_degree(num_nodes):
"""
return num_nodes - 1

def _verify_degree(self, verify):
"""Verify that the number of nodes matches the degree.

Args:
verify (bool): Flag indicating if the degree should be verified
against the number of nodes.

Raises:
ValueError: If ``verify`` is :data:`True` and the number of nodes
does not match the degree.
"""
if not verify:
return

_, num_nodes = self._nodes.shape
expected_nodes = self._degree + 1
if expected_nodes == num_nodes:
return

msg = (
f"A degree {self._degree} curve should have "
f"{expected_nodes} nodes, not {num_nodes}."
)
raise ValueError(msg)

@property
def length(self):
r"""The length of the current curve.
Expand Down Expand Up @@ -167,13 +195,13 @@ class defines ``__slots__`` so by default would not provide a
"_degree": self._degree,
}

def _copy(self):
def copy(self):
"""Make a copy of the current curve.

Returns:
.Curve: Copy of current curve.
"""
return Curve(self._nodes, self._degree, _copy=True)
return Curve(self._nodes, self._degree, copy=True, verify=False)

def evaluate(self, s):
r"""Evaluate :math:`B(s)` along the curve.
Expand Down Expand Up @@ -315,8 +343,8 @@ def subdivide(self):
Tuple[Curve, Curve]: The left and right sub-curves.
"""
left_nodes, right_nodes = _curve_helpers.subdivide_nodes(self._nodes)
left = Curve(left_nodes, self._degree, _copy=False)
right = Curve(right_nodes, self._degree, _copy=False)
left = Curve(left_nodes, self._degree, copy=False, verify=False)
right = Curve(right_nodes, self._degree, copy=False, verify=False)
return left, right

def intersect(
Expand Down Expand Up @@ -443,7 +471,7 @@ def elevate(self):
Curve: The degree-elevated curve.
"""
new_nodes = _curve_helpers.elevate_nodes(self._nodes)
return Curve(new_nodes, self._degree + 1, _copy=False)
return Curve(new_nodes, self._degree + 1, copy=False, verify=False)

def reduce_(self):
r"""Return a degree-reduced version of the current curve.
Expand Down Expand Up @@ -540,7 +568,7 @@ def reduce_(self):
Curve: The degree-reduced curve.
"""
new_nodes = _curve_helpers.reduce_pseudo_inverse(self._nodes)
return Curve(new_nodes, self._degree - 1, _copy=False)
return Curve(new_nodes, self._degree - 1, copy=False, verify=False)

def specialize(self, start, end):
"""Specialize the curve to a given sub-interval.
Expand Down Expand Up @@ -599,7 +627,7 @@ def specialize(self, start, end):
Curve: The newly-specialized curve.
"""
new_nodes = _curve_helpers.specialize_curve(self._nodes, start, end)
return Curve(new_nodes, self._degree, _copy=False)
return Curve(new_nodes, self._degree, copy=False, verify=False)

def locate(self, point):
r"""Find a point on the current curve.
Expand Down
86 changes: 60 additions & 26 deletions src/python/bezier/surface.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,11 @@ class Surface(_base.Base):
degree (int): The degree of the surface. This is assumed to
correctly correspond to the number of ``nodes``. Use
:meth:`from_nodes` if the degree has not yet been computed.
_copy (bool): Flag indicating if the nodes should be copied before
copy (bool): Flag indicating if the nodes should be copied before
being stored. Defaults to :data:`True` since callers may
freely mutate ``nodes`` after passing in.
verify (bool): Flag indicating if the degree should be verified against
the number of nodes. Defaults to :data:`True`.
"""

__slots__ = (
Expand All @@ -189,13 +191,14 @@ class Surface(_base.Base):
"_edges", # Empty default
)

def __init__(self, nodes, degree, _copy=True):
super(Surface, self).__init__(nodes, _copy=_copy)
def __init__(self, nodes, degree, *, copy=True, verify=True):
super(Surface, self).__init__(nodes, copy=copy)
self._degree = degree
self._edges = None
self._verify_degree(verify)

@classmethod
def from_nodes(cls, nodes, _copy=True):
def from_nodes(cls, nodes, copy=True):
"""Create a :class:`.Surface` from nodes.

Computes the ``degree`` based on the shape of ``nodes``.
Expand All @@ -205,7 +208,7 @@ def from_nodes(cls, nodes, _copy=True):
surface. Must be convertible to a 2D NumPy array of floating
point values, where the columns represent each node while the
rows are the dimension of the ambient space.
_copy (bool): Flag indicating if the nodes should be copied before
copy (bool): Flag indicating if the nodes should be copied before
being stored. Defaults to :data:`True` since callers may
freely mutate ``nodes`` after passing in.

Expand All @@ -215,33 +218,58 @@ def from_nodes(cls, nodes, _copy=True):
nodes_np = _base.sequence_to_array(nodes)
_, num_nodes = nodes_np.shape
degree = cls._get_degree(num_nodes)
return cls(nodes_np, degree, _copy=_copy)
# NOTE: **Explicitly** verify because ``_get_degree`` does not.
return cls(nodes_np, degree, copy=copy, verify=True)

@staticmethod
def _get_degree(num_nodes):
"""Get the degree of the current surface.

.. note::

If ``num_nodes`` isn't a triangular number, no degree can be
correct so the return value will be invalid. Callers should use
``verify`` in the constructor to ensure correctness.

Args:
num_nodes (int): The number of control points for a
B |eacute| zier surface.

Returns:
int: The degree :math:`d` such that :math:`(d + 1)(d + 2)/2`
equals ``num_nodes``.

Raises:
ValueError: If ``num_nodes`` isn't a triangular number.
"""
# 8 * num_nodes = 4(d + 1)(d + 2)
# = 4d^2 + 12d + 8
# = (2d + 3)^2 - 1
d_float = 0.5 * (np.sqrt(8.0 * num_nodes + 1.0) - 3.0)
d_int = int(np.round(d_float))
if (d_int + 1) * (d_int + 2) == 2 * num_nodes:
return d_int
return int(np.round(d_float))

else:
raise ValueError(num_nodes, "not a triangular number")
def _verify_degree(self, verify):
"""Verify that the number of nodes matches the degree.

Args:
verify (bool): Flag indicating if the degree should be verified
against the number of nodes.

Raises:
ValueError: If ``verify`` is :data:`True` and the number of nodes
does not match the degree.
"""
if not verify:
return

_, num_nodes = self._nodes.shape
twice_expected_nodes = (self._degree + 1) * (self._degree + 2)
# Avoid rounding by division by 2.
if twice_expected_nodes == 2 * num_nodes:
return

msg = (
f"A degree {self._degree} surface should have "
f"{0.5 * twice_expected_nodes:g} nodes, not {num_nodes}."
)
raise ValueError(msg)

@property
def area(self):
Expand Down Expand Up @@ -302,9 +330,15 @@ def _compute_edges(self):
nodes1, nodes2, nodes3 = _surface_helpers.compute_edge_nodes(
self._nodes, self._degree
)
edge1 = _curve_mod.Curve(nodes1, self._degree, _copy=False)
edge2 = _curve_mod.Curve(nodes2, self._degree, _copy=False)
edge3 = _curve_mod.Curve(nodes3, self._degree, _copy=False)
edge1 = _curve_mod.Curve(
nodes1, self._degree, copy=False, verify=False
)
edge2 = _curve_mod.Curve(
nodes2, self._degree, copy=False, verify=False
)
edge3 = _curve_mod.Curve(
nodes3, self._degree, copy=False, verify=False
)
return edge1, edge2, edge3

def _get_edges(self):
Expand Down Expand Up @@ -353,9 +387,9 @@ def edges(self):
# NOTE: It is crucial that we return copies here. Since the edges
# are cached, if they were mutable, callers could
# inadvertently mutate the cached value.
edge1 = edge1._copy() # pylint: disable=protected-access
edge2 = edge2._copy() # pylint: disable=protected-access
edge3 = edge3._copy() # pylint: disable=protected-access
edge1 = edge1.copy()
edge2 = edge2.copy()
edge3 = edge3.copy()
return edge1, edge2, edge3

@staticmethod
Expand Down Expand Up @@ -751,10 +785,10 @@ def subdivide(self):
nodes_d,
) = _surface_helpers.subdivide_nodes(self._nodes, self._degree)
return (
Surface(nodes_a, self._degree, _copy=False),
Surface(nodes_b, self._degree, _copy=False),
Surface(nodes_c, self._degree, _copy=False),
Surface(nodes_d, self._degree, _copy=False),
Surface(nodes_a, self._degree, copy=False, verify=False),
Surface(nodes_b, self._degree, copy=False, verify=False),
Surface(nodes_c, self._degree, copy=False, verify=False),
Surface(nodes_d, self._degree, copy=False, verify=False),
)

def _compute_valid(self):
Expand Down Expand Up @@ -1110,7 +1144,7 @@ def elevate(self):
# Hold off on division until the end, to (attempt to) avoid round-off.
denominator = self._degree + 1.0
new_nodes /= denominator
return Surface(new_nodes, self._degree + 1, _copy=False)
return Surface(new_nodes, self._degree + 1, copy=False, verify=False)


def _make_intersection(edge_info, all_edge_nodes):
Expand All @@ -1137,7 +1171,7 @@ def _make_intersection(edge_info, all_edge_nodes):
nodes = all_edge_nodes[index]
new_nodes = _curve_helpers.specialize_curve(nodes, start, end)
degree = new_nodes.shape[1] - 1
edge = _curve_mod.Curve(new_nodes, degree, _copy=False)
edge = _curve_mod.Curve(new_nodes, degree, copy=False, verify=False)
edges.append(edge)
return curved_polygon.CurvedPolygon(
*edges, metadata=edge_info, _verify=False
Expand Down
8 changes: 4 additions & 4 deletions tests/functional/test_surface_locate.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@
CONFIG = utils.Config()
# F1 = sympy.Matrix([[s, t]])
SURFACE1 = bezier.Surface.from_nodes(
np.asfortranarray([[0.0, 1.0, 0.0], [0.0, 0.0, 1.0]]), _copy=False
np.asfortranarray([[0.0, 1.0, 0.0], [0.0, 0.0, 1.0]]), copy=False
)
# F2 = sympy.Matrix([[
# (-t**2 + 2 * s + t) / 2, (s**2 + 2 * s * t - s + 2 * t) / 2]])
SURFACE2 = bezier.Surface.from_nodes(
np.asfortranarray(
[[0.0, 0.5, 1.0, 0.25, 0.75, 0.0], [0.0, -0.25, 0.0, 0.5, 0.75, 1.0]]
),
_copy=False,
copy=False,
)
# F3 = sympy.Matrix([[
# -(2 * s * t - 4 * s - t) / 4, (s**2 - s * t + 4 * t) / 4]])
Expand All @@ -39,14 +39,14 @@
[0.0, 0.0, 0.25, 0.5, 0.375, 1.0],
]
),
_copy=False,
copy=False,
)
# F4 = sympy.Matrix([[2 * (s + 2 * t) * (1 - t), 2 * t * (s + 1)]])
SURFACE4 = bezier.Surface.from_nodes(
np.asfortranarray(
[[0.0, 1.0, 2.0, 2.0, 2.0, 0.0], [0.0, 0.0, 0.0, 1.0, 2.0, 2.0]]
),
_copy=False,
copy=False,
)
POINTS = np.asfortranarray(
[[0.0, 0.25, 0.59375, 0.265625, 1.25], [0.0, 0.25, 0.25, 0.73046875, 1.25]]
Expand Down
4 changes: 2 additions & 2 deletions tests/functional/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ class CurveInfo: # pylint: disable=too-few-public-methods
def __init__(self, id_, control_points, implicitized=None, note=None):
self.id_ = id_
self.control_points = control_points
self.curve = bezier.Curve.from_nodes(control_points, _copy=False)
self.curve = bezier.Curve.from_nodes(control_points, copy=False)
self.implicitized = implicitized
self.note = note

Expand Down Expand Up @@ -679,7 +679,7 @@ class SurfaceInfo: # pylint: disable=too-few-public-methods
def __init__(self, id_, control_points, note=None):
self.id_ = id_
self.control_points = control_points
self.surface = bezier.Surface.from_nodes(control_points, _copy=False)
self.surface = bezier.Surface.from_nodes(control_points, copy=False)
self.note = note

@classmethod
Expand Down
Loading