aig-upf / tarski

Tarski - An AI Planning Modeling Framework
Apache License 2.0
59 stars 19 forks source link

C-level output redirection on windows #126

Closed anubhav-cs closed 2 years ago

anubhav-cs commented 2 years ago

Currently, we cannot capture c-level output handler on windows. This causes an error when we when attempt to import command.py. Hence, I cannot ground PDDL on windows.

There are two resolutions for this. The second resolution is what this pull request does.

  1. stdout_redirect is the context manager which requires the c-level output handler c_stdout, but we don't use it anywhere. Hence, we can resolve this by simply removing stdout_redirect and the portion of code which retrieves the handler. That is, we remove everything from https://github.com/aig-upf/tarski/blob/80a33dc5fe6ad2602b40b1c69f758b91188dd8e1/src/tarski/utils/command.py#L72 until end of file.

  2. In case, we want to keep stdout_redirect, I have added the code to retrieve the handler on Windows. The program would fall back to original routine if sys.platform is anything other than win32.

miquelramirez commented 2 years ago

Looks good, thanks @anubhav-cs !

gfrances commented 2 years ago

Thanks @anubhav-cs. I'm having a look at the code. Indeed, as you say, we're not using stdout_redirector anymore. This was used in the past in some integration with an SDD solver that we were exploring, but that we finally dropped. It was probably an oversight on my part that I didn't remove this code.

BTW I'm seeing that the ctypes.CDLL code also caused some problems in mac os in the past. Since we're not using this aanymore, I think I would rather go with your option (1) and remote the entire portion of the code, hence simplifying everything. @miquelramirez , would you be happy with that?

miquelramirez commented 2 years ago

I can't see any obvious side effects, so I would say that 1) seems to be the way to go.

Could you @anubhav-cs ammend the PR with the desired solution? Thanks!

anubhav-cs commented 2 years ago

Hi @miquelramirez

I have updated the PR.

gfrances commented 2 years ago

Thanks Anu!