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

Gen.set_submap! breaks for non-DynamicChoiceMap submaps #516

Open
sritchie opened this issue Nov 7, 2023 · 2 comments
Open

Gen.set_submap! breaks for non-DynamicChoiceMap submaps #516

sritchie opened this issue Nov 7, 2023 · 2 comments

Comments

@sritchie
Copy link

sritchie commented Nov 7, 2023

This problem comes up because there is no interface method for setting values in a choice map. For example:

julia> cm = Gen.choicemap()


julia> Gen.set_submap!(cm, :k, Gen.DynamicDSLChoiceMap(Trie{Any,Gen.ChoiceOrCallRecord}()))


julia> cm
│
└── :k


julia> Gen.set_value!(cm, (:k => :k2), "cake")
ERROR: MethodError: no method matching set_value!(::Gen.DynamicDSLChoiceMap, ::Symbol, ::String)
Closest candidates are:
  set_value!(::DynamicChoiceMap, ::Any, ::Any) at ~/.julia/packages/Gen/ME5el/src/choice_map.jl:741
  set_value!(::DynamicChoiceMap, ::Pair, ::Any) at ~/.julia/packages/Gen/ME5el/src/choice_map.jl:746
Stacktrace:

A fix now would be to put a type on set_submap! so that new_node is only allowed to be a DynamicChoiceMap, or something that supports [] setting.

@sritchie sritchie changed the title Gen.set_submap breaks for non-DynamicChoiceMap submaps Gen.set_submap! breaks for non-DynamicChoiceMap submaps Nov 7, 2023
@fsaad
Copy link
Collaborator

fsaad commented Nov 7, 2023

A fix now would be to put a type on set_submap! so that new_node is only allowed to be a DynamicChoiceMap

A run of the test suite with this proposed fix suggests that the Gen internals currently rely on the ability to invoke Gen.set_submap! on a DynamicDSLChoiceMap, specifically in the following line of code:

set_submap!(discard, key, submap)

The problem now is that the discard, which is a DynamicChoiceMap, may contain a submap that is of type DynamicDSLChoiceMap. If the user tries to call set_value! on the discard choicemap, they will obtain the error from the original post.

@sritchie
Copy link
Author

sritchie commented Nov 7, 2023

@fsaad could that line could change to use merge? Typing this quickly without staring so there could be an obvious problem but that seemed the one clear place in the API for sort-of-mutation on general ChoiceMaps.

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

No branches or pull requests

2 participants