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

External sampler does not work with object positioning #294

Closed
3 tasks done
amolenaar opened this issue Jul 29, 2024 · 5 comments · Fixed by #300
Closed
3 tasks done

External sampler does not work with object positioning #294

amolenaar opened this issue Jul 29, 2024 · 5 comments · Fixed by #300
Labels
enhancement New feature or request

Comments

@amolenaar
Copy link

System Details

  1. Python Version: Python 3.12.4
  2. Scenic Version: Scenic 3.0.0
  3. Operating System / Platform: Apple M2 Air macOS Sonoma 14.5

Detailed Description

For our use case (railway) we want to make sure we create samples in different regions: plains, tunnel, forest area.

To make this work we created an external sampler. Although we can use the external sampler to define values for arbitrary parameters (defined with the with keyword), we cannot assign a region (on/in keyword) from our distribution.

It would be really nice if we can use external parameters to define regions.

For comparison, this is a working scenario:

from scenic_external_sampler.regions import NorthEast, SouthWest

ego = new Object in Uniform(NorthEast, SouthWest)

While this scenario fails:

from scenic_external_sampler.regions import NorthEast, SouthWest
from scenic_external_sampler.sampler import SimpleSampler, Values
param externalSampler = SimpleSampler

ego = new Object in Values(NorthEast, SouthWest)

Steps To Reproduce

Have a look at a sample repository I created: https://github.com/amolenaar/scenic-external-sampler

You can easily set up an environment with Poetry:

poetry install
poetry run pytest

All tests succeed, except test_regions_with_externally_sampled_distribution. This test tries to set a region from an external parameter. It fails, since the external sampler has not been initialized yet.

It shows a stack trace:

.venv/lib/python3.12/site-packages/scenic/syntax/translator.py:105: in scenarioFromString
    return _scenarioFromStream(stream, options, filename, scenario=scenario, **kwargs)
.venv/lib/python3.12/site-packages/scenic/syntax/translator.py:172: in _scenarioFromStream
    compileStream(stream, namespace, compileOptions, filename)
.venv/lib/python3.12/site-packages/scenic/syntax/translator.py:321: in compileStream
    executeCodeIn(code, namespace)
.venv/lib/python3.12/site-packages/scenic/syntax/translator.py:560: in executeCodeIn
    exec(code, namespace)
<string>:1: in <module>
    ???
.venv/lib/python3.12/site-packages/scenic/syntax/veneer.py:1411: in In
    if alwaysProvidesOrientation(region):
.venv/lib/python3.12/site-packages/scenic/syntax/veneer.py:1520: in alwaysProvidesOrientation
    sample = region.sample()
.venv/lib/python3.12/site-packages/scenic/core/distributions.py:149: in sample
    subsamples[child] = child.sample(subsamples)
.venv/lib/python3.12/site-packages/scenic/core/distributions.py:150: in sample
    return self._conditioned.sampleGiven(subsamples)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <scenic_external_sampler.sampler.Values object at 0x13f308350>, value = <DefaultIdentityDict {}>

    def sampleGiven(self, value):
        """Specialization of  `Samplable.sampleGiven` for external parameters.
    
        By default, this method simply looks up the value previously sampled by
        `ExternalSampler.sample`.
        """
>       assert self.sampler is not None
E       AssertionError

.venv/lib/python3.12/site-packages/scenic/core/external_params.py:284: AssertionError

Issue Submission Checklist

  • I am reporting an issue, not asking a question
  • I checked the open and closed issues, forum, etc. and have not found any solution
  • I have provided all necessary code, etc. to reproduce the issue
@amolenaar amolenaar added status: triage Issue needs to be assessed type: bug Something isn't working labels Jul 29, 2024
@Eric-Vin Eric-Vin added enhancement New feature or request and removed type: bug Something isn't working status: triage Issue needs to be assessed labels Aug 2, 2024
@Eric-Vin
Copy link
Collaborator

Eric-Vin commented Aug 2, 2024

Hi Arjan! This is definitely a gap in the current Scenic functionality, but I think there might be a workaround that's not too difficult to implement for now. I would recommend creating a subclass ExternalParameter which wraps the region and who's sampleGiven samples a point from the region as desired. You could then replace in with at (and keep on the same) to get the desired behavior. Hope this helps with your use case, and please let me know if there are issues with this approach!

@amolenaar
Copy link
Author

Hi @Eric-Vin,

What do you mean by "subclass ExternalParameter which wraps the region"?

Something along the lines of:

class Values(ExternalParameter):

    def __init__(self, *values: Any):
        super().__init__()
        self._values = values

    def sampleGiven(self, value):
        if self.sampler is None:
            return self._values[0]
        return super().sampleGiven(value)

Just simply return the first option if no samples have been generated by our custom ExternalSampler?

@dfremont
Copy link
Collaborator

Hi Arjan, the ExternalParameter.sampleGiven method should not get called before the sampler is initialized, so that part is just a bug in Scenic. We'll look into merging at least a quick band-aid fix soon. Eric's suggestion was to have your sampleGiven method call your external sampler to compute a point according to any logic you like, but that's orthogonal to the issue you're having here.

Stepping back a bit, if all you want is to make sure you systematically iterate through all of the regions (rather than picking one at random), VerifAI's grid sampler does that. For example:

param verifaiSamplerType = "grid"
ego = new Object in VerifaiOptions([Region1, Region2])

will pick Region1 for the first scene and Region2 for the second. In your case you'd want to then go back to Region1 and repeat forever (sampling different points from within the regions of course), which you are supposed to be able to do by setting the repeat flag of the grid sampler:

from dotmap import DotMap
param verifaiSamplerType = "grid"
param verifaiSamplerParams = DotMap(repeat=True)
ego = new Object in VerifaiOptions([Region1, Region2])

Unfortunately this doesn't work right now (the repeat flag is ignored and VerifAI raises an exception after both options have been exhausted) due to a bug in the grid sampler which I only just noticed. So we'll work on fixing that too :)

@Eric-Vin Eric-Vin linked a pull request Aug 15, 2024 that will close this issue
4 tasks
@Eric-Vin
Copy link
Collaborator

Eric-Vin commented Aug 15, 2024

The VerifAI solution @dfremont suggested should now function correctly (with latest code on the VerifAI repo's main branch)!

@dfremont
Copy link
Collaborator

@amolenaar Eric pushed a temporary fix for the assertion failure you were getting above: you'll likely get a warning instead, which you can ignore. If the fix doesn't solve your problem, please feel free to reopen this issue.

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

Successfully merging a pull request may close this issue.

3 participants