search

prost-planner / prost

probabilistic planning system for tasks encoded in RDDL
MIT License
41 stars 17 forks source link

Parameter setting combination of MC and PB can lead to a segfault #110

Open geisserf opened 4 years ago

geisserf commented 4 years ago

Running the following command on the current live build produces a segfault after a number of rounds (usually below 10): ./prost.py --release academic-advising_inst_mdp__01 [PROST -ram 0 -s 1 -se [THTS -act [UCB1] -out [MC] -backup [PB] -init [Expand -h [IDS]]]].

I've reproduced this error now on both my laptop and my desktop machine. What I don't understand is that even if switch my revision to one 9 months ago I get a segfault, which makes me wonder whether this a problem on my end, but given that I get the same behaviour on two different machines I can't see how this can be the case.

The segfault happens because action selection is not able to select an action (in the debug build one of the assertions of bestActionIndices.empty() fails), but I don't know yet why this is an issue.

@tkeller @speckdavid Could you run the planner on your end and see if you also get a segfault?

speckdavid commented 4 years ago

@geisserf I can confirm. I have exactly the same behavior on my machine.

thomaskeller79 commented 4 years ago

I suspect the reason is the combination of "-out [MC]" and "-backup [PB]" (which seems to be the only difference to the IPC2014 configuration). I just ran the "UMC" outcome selection that is typically paired with Partial Bellman backups and there was no segfault.

Of course, this is still bad software design. It would be better to reject the combination or remove "UMC" altogether and turn "MC" into "UMC" if paired with a backup function that supports solve labeling. I'd therefore keep this issue open to discuss what to do about it (but maybe rename it accordingly).

geisserf commented 4 years ago

Oh, I did not even notice that I exchanged MC with UMC at some point and reused this old command. Yes, that explains it. I rename the issue in this case.

thomaskeller79 commented 4 years ago

I though this was the only illegal combination of ingredients, but I stumbled over another: The MostPlayedArm recommendation function cannot be combined with a backup function that labels nodes as solved. Currently, this is handled by turining the MostPlayedArm recommendation function into an ExpectedBestArm recommendation function, but this is confusing and can lead to wrong interpretation of results. I'd rather have the planner exit with something like a "conflicting ingredient combination" error in both cases.