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

Ignore stderr from target runner #35

Closed
DE0CH opened this issue Jul 16, 2022 · 7 comments
Closed

Ignore stderr from target runner #35

DE0CH opened this issue Jul 16, 2022 · 7 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@DE0CH
Copy link
Contributor

DE0CH commented Jul 16, 2022

It seems that irace reads from standard error of the target runner (perhaps through redirection of 2>&1). While this is useful for capturing errors and displaying them to the users, it's not very useful for the evaluation because some libraries output warnings to stderr, which do not affect the run but would stop irace.

@MLopez-Ibanez
Copy link
Owner

Hi, this is intentional. The rationale is that irace is very good at finding bugs in the software that is being tuned. Most of the time, we want to alert the user that those bugs exist by stopping irace as soon as something strange is detected. Thus, any message, in stderr/stdout that is not filtered out by the target-runner is potentially some bug. Thus, it is the responsibility of the target-runner to filter any useless messages. My advice would be to always do this filtering selectively to not hide potential bugs, but the user can also decide to redirect stderr to /dev/null or to some temporary file within the target-runner. It is ALWAYS possible for the target-runner to filter any output, while it is not possible for irace to decide what output should be filtered and which output is important.

I hope this makes sense.

@DE0CH
Copy link
Contributor Author

DE0CH commented Jul 19, 2022

TBH, I have somewhat a different opinion. In my case it was a warning of division by zero from an external library. I checked to make sure my code was correct so I guess it's just a bug in the external library, which was handled (because the program did not crash). I just made my program swallow the stderr to fix it.

Sometimes programmers choose to print handled exceptions and other non-crucial warnings into stderr. So I think if the program exits with code 0, and prints a number, it doesn't have an unrecoverable bug. Crucial Exceptions would usually travel up the stack and crash the program if not handled instead of just printing something to stderr.

Reading stderr would also not detect logic bugs. So overall I think reading from std would not really alert users to potential problems but create a slight inconvenience.

I think instead of asking the user to redirect stderr, it should expect them to redirect stderr to stdout if want to be extra diligent.

@MLopez-Ibanez
Copy link
Owner

TBH, I have somewhat a different opinion. In my case it was a warning of division by zero from an external library. I checked to make sure my code was correct so I guess it's just a bug in the external library, which was handled (because the program did not crash). I just made my program swallow the stderr to fix it.

This is the right solution in my opinion (except that it should not swallow the complete stderr but only that particular warning or at least it should save stderr to some file to inspect it later). You made sure that your code was correct and you changed your program (or the target-runner) to ignore it. If your code was not correct and irace did not report the error, it would be quite hard to track it.

Sometimes programmers choose to print handled exceptions and other non-crucial warnings into stderr. So I think if the program exits with code 0, and prints a number, it doesn't have an unrecoverable bug. Crucial Exceptions would usually travel up the stack and crash the program if not handled instead of just printing something to stderr.

Unfortunately, irace cannot assume that this is true in general. Ignoring stderr is a decision that must be made by the user. irace cannot make this decision for the user. If there was something printed in stderr and the user did not expect it, it seems worth it to report it to the user and let the user decide what to do.

Reading stderr would also not detect logic bugs. So overall I think reading from std would not really alert users to potential problems but create a slight inconvenience.

"Perfect is the enemy of good" :-) Even if it cannot catch everything, it will catch some things.

I think there are two possible extremes:

  1. irace ignores everything in the output except for some "field" like "Output for irace: 1.25". This would be a bit more convenient, since users will not have to remove anything from the output and only need to add this line.
  2. irace complains about anything in the output (stdout+stderr) that is not "1.25". This creates a slight inconvenience because the user needs to filter any output that should be ignored.

I see advantages and disadvantages for both:

  1. more convenient but possibly slower (irace has to save and possibly parse all the output to find the correct output) and will ignore any warnings, errors or informative messages that still give the special field.
  2. less convenient but possibly faster (irace only has to parse 1 single number) and will detect any unexpected warnings, errors or informative messages.

Option 1 is the one implemented by the AClib wrapper. You can enable it using --aclib (but then you have to write your target-runner to conform to the aclib format).

Option 2 is the one used by default by irace to make sure we detect as many potential errors as possible.

I think instead of asking the user to redirect stderr, it should expect them to redirect stderr to stdout if want to be extra diligent.

I see a possible option 3: irace saves stderr to a temporary file and only reports its contents if the evaluation fails. If the file is empty, it is deleted. If the file is not empty, it leaves it there for the user to see if they want to check what is inside. I will be happy if somebody implemented this option.

@DE0CH
Copy link
Contributor Author

DE0CH commented Jul 20, 2022

I see. That is very well thought out. Thanks for the thorough opinion. I agree with you now.

@DE0CH DE0CH closed this as completed Jul 20, 2022
@TheIronBorn
Copy link

option 3 would be very useful. Users could implement their own intermediate step but it is often the case they won't consider they'll need to until they hit an unexpected error

@MLopez-Ibanez
Copy link
Owner

option 3 would be very useful. Users could implement their own intermediate step but it is often the case they won't consider they'll need to until they hit an unexpected error

I'm happy to review any pull-request adding this option. It would complement another option that I haven't finished that tells irace to retry a failed evaluation a number of times before giving up. This is useful for flaky systems that sometimes fail due to hardware problems.

@MLopez-Ibanez
Copy link
Owner

I have opened #42 to track option 3 so we can close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants