search

MiniZinc / mzn-bench

A framework to performing benchmark testing on MiniZinc models, solvers, and/or the compiler itself.
https://www.minizinc.org
5 stars 5 forks source link

Show only feasible results in plots #7

Closed hbierlee closed 2 years ago

hbierlee commented 3 years ago

I'm not sure if this is the best way to handle this, but something must be done as ERROR results might miss the time stat, and UNKNOWN results might have a time equal to the timeout value which is misleading (we didn't get a solution at the timeout, after all)

Dekker1 commented 3 years ago

To me it seems like this at least wouldn't be the correct place, as the function is really just intended to read the CSV data.

It sounds like you are finding results with these statuses to be a problem in the plotting function, so I would suggest that we would deal with it there. @cyderize would probably know more though.

cyderize commented 3 years ago

I had originally purposely allowed results without solutions to still get plotted - hovering would tell you the status. I would have considered it to be the user's responsibility to filter out UNKNOWN results if they didn't want to plot them (they're using a function to plot instances - and that's what it's going to do, regardless of if there were solutions or not).

You're right about ERROR statuses. Where there's no time at all, it can't be plot, although I think that should probably just be an error (and so again you'd have to filter it out yourself). Although now that I think about it, wouldn't collect_statistics itself fail if it's missing the time? We should probably fix that if that's the case.

I'd like to avoid just silently ignoring stuff without telling the user if we can help it, since I think that's even more dangerous a lot of the time.

hbierlee commented 3 years ago

Yes, agreed, and come to think of it, this change also filters UNSAT for which we definitely do want to show the time and everything. And even ERROR statuses might have a time stat which would be good to show (e.g., the time at which the error ocurred).

Btw, collect_statistics won't fail, it will just have an empty string in the time column if there is none.

But I do think there should be some visual distinction between these statuses, because a solution at a minute before countdown is very different from an UNKNOWN and hovering over every hbar will be cumbersome.

Personally, I think a pretty intuitive and easy solution would be to just change the fill pattern (hatch pattern?). Then there is no ambiguity, and every run/configuration is in every plot:

Dekker1 commented 3 years ago

Any update on what is happening with this PR?

hbierlee commented 3 years ago

Yes, Jason and I agreed that a hatch pattern was probably the way to go (unless there's a new idea in the meantime). I'll update the PR today so you can check it out!

On Thu, May 6, 2021 at 2:16 PM Jip J. Dekker @.***> wrote:

Any update on what is happening with this PR?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MiniZinc/mzn-bench/pull/7#issuecomment-833211460, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABY43XI2LGSUPFAQ4TIEJJTTMIJ2BANCNFSM4X5NAFTA .