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

Visible/Non Visible Specifier Changes #214

Merged
merged 33 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
06f1335
Visibility specifier changes to no longer underapproximate.
Eric-Vin Feb 2, 2024
dc30c08
Updated visible specifiers docs.
Eric-Vin Feb 2, 2024
642f651
Fixes and pitch tweaks.
Eric-Vin Feb 2, 2024
1c1b49b
Merge branch 'main' into VisibleSpecifierChanges
Eric-Vin Feb 6, 2024
6cdc455
Fixes and test tweaks for new semantics.
Eric-Vin Feb 7, 2024
6c6d72f
Fixed checkCyclical.
Eric-Vin Feb 16, 2024
d646b69
Progress on pruning tweaks.
Eric-Vin Feb 17, 2024
6cfef98
Visiblity pruning progress.
Eric-Vin Feb 22, 2024
4ff8ab7
Work on Voxel to Mesh conversion.
Eric-Vin Feb 28, 2024
9ebfe93
More visible spec changes.
Eric-Vin Feb 28, 2024
60e3dac
Voxel to Mesh Workaround
Eric-Vin Mar 15, 2024
801f179
Documentation edits.
Eric-Vin Mar 18, 2024
1002c47
Update src/scenic/core/regions.py
Eric-Vin Mar 18, 2024
af6e5e7
Merge branch 'VisibleSpecifierChanges' of github.com:BerkeleyLearnVer…
Eric-Vin Mar 18, 2024
b499299
Test cleanup
Eric-Vin Mar 18, 2024
e7aea48
Test tweaks.
Eric-Vin Mar 18, 2024
f6288f8
Add WIP warnings to VoxelRegion.
Eric-Vin Mar 18, 2024
075c458
Merge branch 'main' into VisibleSpecifierChanges
Eric-Vin Mar 18, 2024
b3905ff
Added test for visibility pruning with offset.
Eric-Vin Mar 25, 2024
66ef021
Ensure test to ensure offset is preserved after visibility pruning.
Eric-Vin Mar 25, 2024
d54e9b7
Added validation tests.
Eric-Vin Mar 25, 2024
d9c27fa
Added test for intersection region sampler.
Eric-Vin Mar 26, 2024
2f3e359
Test modification for extra coverage.
Eric-Vin Mar 26, 2024
074364c
Generic sampling tests/fixes.
Eric-Vin Mar 26, 2024
11d17e1
Various requested fixes.
Eric-Vin Apr 16, 2024
341f6be
Apply suggestions from code review
Eric-Vin Apr 16, 2024
37f6f87
Docs updates.
Eric-Vin Apr 16, 2024
fd26356
Merge branch 'VisibleSpecifierChanges' of github.com:BerkeleyLearnVer…
Eric-Vin Apr 16, 2024
8c58c25
Update tests/syntax/test_specifiers.py
Eric-Vin Apr 18, 2024
df2aa8a
Visibility test cleanup
Eric-Vin Apr 18, 2024
e6c3fdb
Voxel mesh cleanup and test.
Eric-Vin Apr 18, 2024
5b870ed
Voxel to mesh cleanup.
Eric-Vin Apr 18, 2024
35fa757
Modified pitch_signs formatting.
Eric-Vin Apr 19, 2024
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
2 changes: 2 additions & 0 deletions docs/new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ Backwards-incompatible semantics changes:

* The :specifier:`offset by` specifier now optionally specifies :prop:`parentOrientation`.

* The :specifier:`visible` and :specifier:`not visible` specifiers now take into account occlusion and the shapes of objects. In previous versions, they only checked whether the center of the object was visible/not visible, ignoring occlusion.

Backwards-incompatible API changes:

* The **maxIterations** argument of `Simulator.simulate` now has default value 1, rather than 100.
Expand Down
2 changes: 2 additions & 0 deletions docs/porting.rst
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,7 @@ Specifically:

* The specifier :specifier:`with heading {X}` is replaced with :specifier:`facing {X}`.

* The :specifier:`visible` and :specifier:`not visible` will behave as they did in Scenic 2, requiring the *center* of the object to be visible rather than *any* part of the object. More precisely, :specifier:`visible` will specify :prop:`position` to be uniformly random in the observing object's :term:`visible region` and :specifier:`not visible` will specify :prop:`position` to be uniformly random in the difference of the :term:`workspace` and the observing object's visible region.

Note that despite these changes, Scenic will still use 3D geometry internally.
For example, if you write :scenic:`ego = new Object at (1, 2)` the value of :scenic:`ego.position` will be the 3D vector :scenic:`(1, 2, 0)`.
20 changes: 15 additions & 5 deletions docs/reference/specifiers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,19 @@ visible [from (*Point* | *OrientedPoint*)]
* :prop:`position` with priority 3
* also adds a requirement (see below)

**Dependencies**: None
**Dependencies**: :prop:`regionContainedIn`

Requires that this object is visible from the :scenic:`ego` or the given `Point`/`OrientedPoint`. See the :ref:`Visibility System <visibility>` reference for a discussion of the visibility model.

Also optionally specifies :prop:`position` to be a uniformly random point in the :term:`visible region` of the ego, or of the given Point/OrientedPoint if given.
Note that the position set by this specifier is slightly stricter than simply adding a requirement that the ego :keyword:`can see` the object: the specifier makes the *center* of the object (its :prop:`position`) visible, while the :keyword:`can see` condition will be satisfied even if the center is not visible as long as some other part of the object is visible.
Also optionally specifies :prop:`position` to be uniformly random over all points that could result in a visible object (note that the above requirement will ensure the object is in fact visible).

.. versionchanged:: 3.0

This specifier now specifies :prop:`position` uniformly randomly over all points that could result in a visible object. This allows for objects whose :prop:`position` might be out of the visible region, but which have a portion of their occupied space visible (e.g. a corner that is visible). With the previous semantics, such configurations would never be generated because the *center* of the object was required to be visible.

.. note::

As an implementation detail, :prop:`position` is initially set to be sampled from `everywhere` (or the :term:`workspace` if one has been set). Scenic will then attempt to further restrict the sample region via various pruning techniques, but sometimes this is not possible. If this occurs and Scenic has not been able to further restrict the sampled region from `everywhere`, an error will be raised at compile time. The simplest way to remedy this is by setting a workspace or specifying :prop:`position` with a higher priority using a different specifier.

.. _not visible [from ({Point} | {OrientedPoint})]:

Expand All @@ -209,8 +216,11 @@ not visible [from (*Point* | *OrientedPoint*)]

Requires that this object is *not* visible from the ego or the given `Point`/`OrientedPoint`.

Similarly to :sampref:`visible [from ({Point} | {OrientedPoint})]`, this specifier can position the object uniformly at random in the *non-visible* region of the ego.
However, it depends on :prop:`regionContainedIn`, in order to restrict the non-visible region to the :term:`container` of the object being created, which is hopefully a bounded region (if the non-visible region is unbounded, it cannot be uniformly sampled from and an error will be raised).
Similarly to :sampref:`visible [from ({Point} | {OrientedPoint})]`, this specifier can optionally position the object uniformly at random over all points that could result in a non-visible object (note that the above requirement will ensure the object is in fact not visible).

.. versionchanged:: 3.0

This specifier now specifies :prop:`position` uniformly randomly over all points that could result in a non-visible object. This disallows objects whose :prop:`position` is out of the visible region, but which have a portion of their occupied space visible (e.g. a corner that is visible). With the previous semantics, such configurations would sometimes be generated because only the *center* of the object was required to be non-visible.

.. _(left | right) of {vector} [by {scalar}]:
.. _left of:
Expand Down
4 changes: 2 additions & 2 deletions docs/syntax_guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,9 @@ Additional specifiers for the :prop:`position` and :prop:`orientation` propertie
* - :sampref:`beyond {vector} by ({vector} | {scalar}) [from ({vector} | {OrientedPoint})]`
- Positions the object with respect to the line of sight from a point or the ego
* - :sampref:`visible [from ({Point} | {OrientedPoint})]`
- Ensures the object is visible from the ego, or from the given Point/OrientedPoint if given, while optionally specifying position to be in the appropriate visible region.
- Ensures the object is visible from the ego, or from the given Point/OrientedPoint if given, while optionally specifying position to be uniformly random over all positions that result in a visible object.
* - :sampref:`not visible [from ({Point} | {OrientedPoint})]`
- Ensures the object is not visible from the ego, or from the given Point/OrientedPoint if given, while optionally specifying position to be outside the appropriate visible region.
- Ensures the object is not visible from the ego, or from the given Point/OrientedPoint if given, while optionally specifying position to be uniformly random over all positions that result in a non-visible object.
* - :sampref:`(left | right) of ({vector} | {OrientedPoint} | {Object}) [by {scalar}] <left of>`
- Positions the object to the left/right by the given scalar distance.
* - :sampref:`(ahead of | behind) ({vector} | {OrientedPoint} | {Object}) [by {scalar}] <ahead of>`
Expand Down
45 changes: 27 additions & 18 deletions src/scenic/core/distributions.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,8 @@ def __getattr__(self, name):
return object.__getattribute__(self, name)
return AttributeDistribution(name, self)

def __call__(self, *args):
return OperatorDistribution("__call__", self, args)
def __call__(self, *args, **kwargs):
return OperatorDistribution("__call__", self, args, kwargs)

def __iter__(self):
raise RandomControlFlowError(f"cannot iterate through a random value")
Expand Down Expand Up @@ -684,7 +684,7 @@ def supportInterval(self):
return unionOfSupports(supportInterval(attr) for attr in attrs)
return None, None

def __call__(self, *args):
def __call__(self, *args, **kwargs):
vty = self.object._valueType
retTy = None
if vty is not object:
Expand All @@ -693,7 +693,7 @@ def __call__(self, *args):
if isinstance(func, property):
func = func.fget
retTy = get_type_hints(func).get("return")
return OperatorDistribution("__call__", self, args, valueType=retTy)
return OperatorDistribution("__call__", self, args, kwargs, valueType=retTy)

def __repr__(self):
return f"{self.object!r}.{self.attribute}"
Expand All @@ -704,15 +704,18 @@ class OperatorDistribution(Distribution):

_deterministic = True

def __init__(self, operator, obj, operands, valueType=None):
def __init__(self, operator, obj, operands, kwoperands, valueType=None):
operands = tuple(toDistribution(arg) for arg in operands)
kwoperands = {key: toDistribution(kwarg) for key, kwarg in kwoperands.items()}
dependencies = operands + tuple(kwoperands.values())
if valueType is None:
ty = type_support.underlyingType(obj)
valueType = self.inferType(ty, operator, operands)
super().__init__(obj, *operands, valueType=valueType)
valueType = self.inferType(ty, operator, operands, kwoperands)
super().__init__(obj, *dependencies, valueType=valueType)
self.operator = operator
self.object = obj
self.operands = operands
self.kwoperands = kwoperands
self.symbol = allowedReversibleOperators.get(operator)
if self.symbol:
if operator[:3] == "__r":
Expand All @@ -723,7 +726,7 @@ def __init__(self, operator, obj, operands, valueType=None):
self.reverse = None

@staticmethod
def inferType(ty, operator, operands):
def inferType(ty, operator, operands, kwoperands):
"""Attempt to infer the result type of the given operator application."""
# If the object's type is known, see if we have a return type annotation.
origin = type_support.get_type_origin(ty)
Expand All @@ -741,7 +744,9 @@ def inferType(ty, operator, operands):
# None does not support this operator; using it will raise an
# exception, so we can ignore this case for type inference.
continue
res = OperatorDistribution.inferType(option, operator, operands)
res = OperatorDistribution.inferType(
option, operator, operands, kwoperands
)
types.append(res)
return type_support.unifierOfTypes(types) if types else object

Expand Down Expand Up @@ -796,10 +801,11 @@ def scalar(thing):
def sampleGiven(self, value):
first = value[self.object]
rest = [value[child] for child in self.operands]
kwargs = {key: value[child] for key, child in self.kwoperands.items()}
op = getattr(first, self.operator)
result = op(*rest)
result = op(*rest, **kwargs)
if result is NotImplemented and self.reverse:
assert len(rest) == 1
assert len(rest) == 1 and len(kwargs) == 0
rop = getattr(rest[0], self.reverse)
result = rop(first)
if result is NotImplemented and self.symbol:
Expand All @@ -812,7 +818,10 @@ def sampleGiven(self, value):
def evaluateInner(self, context):
obj = valueInContext(self.object, context)
operands = tuple(valueInContext(arg, context) for arg in self.operands)
return OperatorDistribution(self.operator, obj, operands)
kwoperands = {
key: valueInContext(arg, context) for key, kwarg in self.kwoperands.items()
}
return OperatorDistribution(self.operator, obj, operands, kwoperands)

def supportInterval(self):
if self.operator in (
Expand All @@ -825,7 +834,7 @@ def supportInterval(self):
"__truediv__",
"__rtruediv__",
):
assert len(self.operands) == 1
assert len(self.operands) == 1 and len(self.kwoperands) == 0
l1, r1 = supportInterval(self.object)
l2, r2 = supportInterval(self.operands[0])
if l1 is None or l2 is None or r1 is None or r2 is None:
Expand Down Expand Up @@ -859,7 +868,7 @@ def supportInterval(self):
raise AssertionError(f"unexpected operator {self.operator}")
return l, r
elif self.operator in ("__neg__", "__abs__"):
assert len(self.operands) == 0
assert len(self.operands) == 0 and len(self.kwoperands) == 0
l, r = supportInterval(self.object)
if self.operator == "__neg__":
return -r, -l
Expand Down Expand Up @@ -921,7 +930,7 @@ def handler(self, arg):
and arg == 0
):
return self
return OperatorDistribution(op, self, (arg,), valueType=ty)
return OperatorDistribution(op, self, (arg,), {}, valueType=ty)

elif op in ("__mul__", "__rmul__"):

Expand All @@ -933,7 +942,7 @@ def handler(self, arg):

if issubclass(self._valueType, Orientation) and arg == globalOrientation:
return self
return OperatorDistribution(op, self, (arg,), valueType=ty)
return OperatorDistribution(op, self, (arg,), {}, valueType=ty)

elif op in ("__truediv__", "__floordiv__", "__pow__"):

Expand All @@ -944,12 +953,12 @@ def handler(self, arg):
and arg == 1
):
return self
return OperatorDistribution(op, self, (arg,), valueType=ty)
return OperatorDistribution(op, self, (arg,), {}, valueType=ty)

else:
# The general case.
def handler(self, *args):
return OperatorDistribution(op, self, args, valueType=ty)
return OperatorDistribution(op, self, args, {}, valueType=ty)

return handler

Expand Down
2 changes: 1 addition & 1 deletion src/scenic/core/lazy_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ def needsLazyEvaluation(thing):

def dependencies(thing):
"""Dependencies which must be sampled before this value."""
return getattr(thing, "_dependencies", ())
return getattr(getattr(thing, "_conditioned", thing), "_dependencies", ())


def needsSampling(thing):
Expand Down
Loading
Loading