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

Dead ALEX after requesting departure_time without necessary info #165

Open
michlikv opened this issue May 26, 2015 · 6 comments
Open

Dead ALEX after requesting departure_time without necessary info #165

michlikv opened this issue May 26, 2015 · 6 comments

Comments

@michlikv
Copy link
Contributor

Dialogue:
s: hello()
u: request(departure_time)
s: ...
u: inform(alternative=2)
s: Dead, there is no connection info.

After we request departure_time or departure_time_rel, it probably somehow creates 'route_alternative' slot in DState "slots" -- with value = none, even though the user did not provide enough info to find connection.
Then when I ask for any alternative, it crashes.

Using: hdc_policy with google info.
(hopefully this is the right place to put this :))

@tuetschek
Copy link
Member

Yes, this is the right place, thanks. I'll look at it.

@tuetschek
Copy link
Member

OK, this is actually a result of a deeper inconsistency...

@jurcicek: Why is dialogue_state['route_alternative'] initially a D3DiscreteValue like the other slots, even though this is not used as a regular slot? You added it to the list of slots in ontology.py in 90ac9d0 .

The variable is initialy an empty D3DiscreteValue, but it is changed to an int when directions are found. This is very counter-intuitive. I think that initially it should be None, or – perhaps better – not present in the dialogue state variable at all, same as conn_info (i.e., a special member variable of the dialogue state, not in its slot dictionary).

@tuetschek
Copy link
Member

Actually, now it seems to me that we should separate conn_info and route_alternative from the dialogue state completely and keep them as member variables of PTICSHDCPolicy...

Or we should introduce a way to initialize them in DeterministicDiscriminativeDialogueState at the start of the dialogue – otherwise they are not there at the beginning and we have to test for them using hasattr, which is rather ugly.

@jurcicek
Copy link
Member

DeterministicDiscriminativeDialogueState automatically create all accessed
slots as D3DiscreteValue initially. You can change them as you wish later.

The automatic creation was chosen because I did not want to use hasattr.

Best regards,
Filip Jurcicek


Work tel. (CZ): +420221914402
Personal tel. (CZ): +420777805048
Skype: bozskyfilip

http://ufal.mff.cuni.cz/filip-jurcicek

On 27 May 2015 at 11:36, Ondrej Dusek [email protected] wrote:

OK, this is actually a result of a deeper inconsistency...

@jurcicek https://github.com/jurcicek: Why is
dialogue_state['route_alternative'] initially a D3DiscreteValue like the
other slots, even though this is not used as a regular slot? You added it
to the list of slots in ontology.py in 90ac9d0
90ac9d0
.

The variable is initialy an empty D3DiscreteValue, but it is changed to
an int when directions are found. This is very counter-intuitive. I think
that initially it should be None, or – perhaps better – not present in
the dialogue state variable at all, same as conn_info (i.e., a special
member variable of the dialogue state, not in its slot dictionary).


Reply to this email directly or view it on GitHub
#165 (comment).

@jurcicek
Copy link
Member

On 27 May 2015 at 11:41, Ondrej Dusek [email protected] wrote:

Actually, now it seems to me that we should separate conn_info and
route_alternative from the dialogue state completely and keep them as
member variables of PTICSHDCPolicy...

Conceptually, I want to keep them as a part of the state. The dialogue
state is the place where all information should be stored. The policy
selects actionsbased on the information stored in the state.

Or we should introduce a way to initialize them in
DeterministicDiscriminativeDialogueState at the start of the dialogue –
otherwise they are not there at the beginning and we have to test for them
using hasattr, which is rather ugly.

You mean all variables or only the conn_info and route_alternative
variables?


Reply to this email directly or view it on GitHub
#165 (comment).

@tuetschek
Copy link
Member

I mean only conn_info and route_alternative – these relate to the current dialogue state, but are not used as slots (you don't need probabilities there). Are there any others that work this way?

Yes, I know that I can change them to whatever I want later, but calling isinstance on them (which is what we do now) seems to me almost as ugly as hasattr. Using them as a D3DiscreteValue would not make much sense either since they're not real probabilities.

I think that we can give a list of these variables in the ontology – they will then be initialized by the DeterministicDiscriminativeDialogueState constructor (to None, not D3DiscreteValue) and we won't need hasattr.

tuetschek added a commit that referenced this issue May 29, 2015
The `dialogue_state` variable is now a member variable of the state,
(same as `conn_info` and `directions`), not a slot.

This reflects the actual usage of the variable – it is always integer,
never a probability distribution. Now wo don't need to use `isinstance`
to test whether a route has been found, we can use `is not None`, which
is nicer.

This fixes the problem reported in #165.
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

3 participants