search

Kipok / NeMo-Skills

A pipeline to improve skills of large language models
https://kipok.github.io/NeMo-Skills/
Apache License 2.0
185 stars 41 forks source link

Modify sandbox code execution to capture stderr #98

Closed aleksficek closed 2 months ago

aleksficek commented 2 months ago

Can we make this change so that both the stdout and stderr outputs are captured in sandbox code execution? This also resembles the return object from Piston. Additionally, this makes code execution when the code fails much faster as it does not wait for timeout to occur and fails immediately.

I'm not sure what NeMo-Skills code will break with this change so let me know what uses this and I can make the changes if needed.

Kipok commented 2 months ago

Yeah, this will break a bunch of downstream code. But you can already get error message if you use the Sandbox class (which you should be using as it also provides a unified interface between piston and local exec) https://github.com/Kipok/NeMo-Skills/blob/main/nemo_skills/code_execution/sandbox.py#L224. Here are some examples how you can use it https://github.com/Kipok/NeMo-Skills/blob/main/tests/test_code_execution.py#L39

Also please note that this logic is in the process of being refactored and eventually (in a couple of weeks) we will switch to this version that does provide you stdout/stderr explicitly https://github.com/Kipok/NeMo-Skills/pull/89. But hopefully for your purposes the current sandbox api is already good enough (it shouldn't wait for timeout btw, if the code has an error, it will finish immediately). Let me know if you still need to make some changes, happy to discuss that

aleksficek commented 2 months ago

Yeah, this will break a bunch of downstream code. But you can already get error message if you use the Sandbox class (which you should be using as it also provides a unified interface between piston and local exec) https://github.com/Kipok/NeMo-Skills/blob/main/nemo_skills/code_execution/sandbox.py#L224. Here are some examples how you can use it https://github.com/Kipok/NeMo-Skills/blob/main/tests/test_code_execution.py#L39

Also please note that this logic is in the process of being refactored and eventually (in a couple of weeks) we will switch to this version that does provide you stdout/stderr explicitly #89. But hopefully for your purposes the current sandbox api is already good enough (it shouldn't wait for timeout btw, if the code has an error, it will finish immediately). Let me know if you still need to make some changes, happy to discuss that

Got it thanks for clearing that up. So I have something using my own local custom setup currently working, I'll merge and use that until the refactors come in and then switch NeMo-Codegen to the new code in NeMo-Skills for consistency. Ping me on slack when the changes get in and I can make the updates.

Kipok commented 2 months ago

sounds good, thanks @aleksficek. Let me close this for now then