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

Cannot record values in sub-scenarios of a modular scenario #161

Open
abol-karimi opened this issue Jul 26, 2023 · 3 comments
Open

Cannot record values in sub-scenarios of a modular scenario #161

abol-karimi opened this issue Jul 26, 2023 · 3 comments
Assignees
Labels
type: bug Something isn't working

Comments

@abol-karimi
Copy link

The simulation result of a modular scenario does not contain the records dictionary of the records statements in the setup sections of its sub-scenarios.

@Eric-Vin Eric-Vin added the type: bug Something isn't working label Mar 25, 2024
@Eric-Vin
Copy link
Collaborator

Can reproduce this behavior with the following example:

param map = localPath('assets/maps/CARLA/Town05.xodr')
param carla_map = 'Town05'
param time_step = 1.0/10

model scenic.domains.driving.model

scenario Sub():
    setup:
        record "FOO" as "foo"

scenario Main():
    setup:
        ego = new Car
        record "BAR" as "bar"
    compose:
        do Sub()

@dfremont
Copy link
Collaborator

The root issue is that I didn't expect record to be used anywhere except at the top level. We could extend it to support sub-scenarios, but the best semantics isn't totally clear to me. The difficulty is that if multiple copies of the sub-scenario are invoked, the records will conflict: for example if the compose block above were do Sub(), Sub() (two copies of Sub in parallel) or do Sub(); do Sub() (two copies in sequence). I think the least surprising behavior for plain record would be for Scenic to raise an error in the parallel case and concatenate the time series in the sequential case. @abol-karimi would that work for your use case?

For record initial and record final, I can't see any way to make those work in a backwards-compatible way when the scenario is invoked multiple times, so I think we should just raise an error. I could see potential applications for saving a value each time a sub-scenario is invoked, but since the result would still be a time series, it wouldn't fit with the documented behavior of record initial/final as producing a single value in the records dictionary. (I suppose we could have the value be a time series if and only if the sub-scenario runs multiple times, but all user code reading the record would have to check for both cases, so I'm not sure if it's worth the complexity.)

@abol-karimi
Copy link
Author

It works perfectly fine for me (i.e. runtime error for parallel, concatenation for serial.) My use case for modular scenarios is mainly re-usability and managing the complexity, and I don't invoke multiple instances of a scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants