siemens / OOASP

MIT License
7 stars 0 forks source link

Nico/benchmarks #58

Closed susuhahnml closed 2 weeks ago

susuhahnml commented 3 weeks ago

Hi @Kriplingo! Here is our update. There is a timeout parameter in the command line that we can use so that clingo is properly stoped and the results are still printed. Running the command line (without verbose) will print only the json output with the information. So you can also just save it with > By default no smart functions are used. You can add them in a single string: --smart-generation "object_needed,global_ub,global_lb,association_needed" The idea would be to vary the order of this functions and the initial objects on the benchmarks. Let us know if this works in your setting and if this gives you interesting results.

Kriplingo commented 3 weeks ago

Hi @Kriplingo! Here is our update. There is a timeout parameter in the command line that we can use so that clingo is properly stoped and the results are still printed. Running the command line (without verbose) will print only the json output with the information. So you can also just save it with > By default no smart functions are used. You can add them in a single string: --smart-generation "object_needed,global_ub,global_lb,association_needed" The idea would be to vary the order of this functions and the initial objects on the benchmarks. Let us know if this works in your setting and if this gives you interesting results.

@susuhahnml these are great changes, but I have ran into a bug, where if the --timeout argument is not provided, the program crashes with the following error message:

Traceback (most recent call last):
  File "C:\Users\z004zdbc\Desktop\fresh\OOASP\ooasp\run.py", line 566, in <module>
    solver.run()
  File "C:\Users\z004zdbc\Desktop\fresh\OOASP\ooasp\run.py", line 521, in run
    time_left = self.timeout - (time.time() - run_start)
                ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
TypeError: unsupported operand type(s) for -: 'NoneType' and 'float'

I am quite certain this is unwanted behaviour, and we should either set a default (perhaps an infinity constant), or change the code to avoid this situation. Please let me know if I should attempt a fix.

Kriplingo commented 3 weeks ago

@susuhahnml @nrueh also I don't think the timeouts are working properly, either I am calling it wrong, or there is some mistake when it comes to terminating the solving early. python ooasp/run.py --elementA 4 --elementB 4 --elementC 4 --elementD 4 --timeout 1 --smart-generation "association_needed" command has been running on my computer for about 20 minutes now. Am I calling it wrong?

nrueh commented 3 weeks ago

@susuhahnml @nrueh also I don't think the timeouts are working properly, either I am calling it wrong, or there is some mistake when it comes to terminating the solving early. python ooasp/run.py --elementA 4 --elementB 4 --elementC 4 --elementD 4 --timeout 1 --smart-generation "association_needed" command has been running on my computer for about 20 minutes now. Am I calling it wrong?

Hm, that is weird. I ran your command and the first time it worked but now it also does not seem to terminate. Let me have a look

nrueh commented 3 weeks ago

@susuhahnml @nrueh also I don't think the timeouts are working properly, either I am calling it wrong, or there is some mistake when it comes to terminating the solving early. python ooasp/run.py --elementA 4 --elementB 4 --elementC 4 --elementD 4 --timeout 1 --smart-generation "association_needed" command has been running on my computer for about 20 minutes now. Am I calling it wrong?

I fixed this, too, I think by breaking out of the loop before any solving is started in case the timeout is already reached. Currently, the timeout also does not affect the smart generation. I will think about making it more general.

Kriplingo commented 3 weeks ago

@susuhahnml @nrueh also I don't think the timeouts are working properly, either I am calling it wrong, or there is some mistake when it comes to terminating the solving early. python ooasp/run.py --elementA 4 --elementB 4 --elementC 4 --elementD 4 --timeout 1 --smart-generation "association_needed" command has been running on my computer for about 20 minutes now. Am I calling it wrong?

I fixed this, too, I think by breaking out of the loop before any solving is started in case the timeout is already reached. Currently, the timeout also does not affect the smart generation. I will think about making it more general.

@nrueh One more question, as I am currently testing it further. Should the timeout apply to grounding as well? Because currently even in the new version, it does not seem to affect grounding: python ooasp/run.py --elementA 20 --elementB 20 --elementC 20 --elementD 20 --timeout 1 does not seem to terminate on timeout and gives this result (which does not seem correct if it should apply to grounding, and if it weren't to, I don't quite understand why solving would not even start):

{
    "timeout": true,
    "#objects": 80,
    "#objects_added_per_type": {
        "elementA": 20,
        "elementB": 20,
        "elementC": 20,
        "elementD": 20
    },
    "#associations": 0,
    "times": {
        "runtime": 41.895,
        "initialization": 41.895,
        "smart_generation": {
            "time": 0.0,
            "cautious": 0,
            "brave": 0,
            "functions": {}
        },
        "solve": 0,
        "ground": 41.891
    },
    "model": null
}
nrueh commented 3 weeks ago

@susuhahnml @nrueh also I don't think the timeouts are working properly, either I am calling it wrong, or there is some mistake when it comes to terminating the solving early. python ooasp/run.py --elementA 4 --elementB 4 --elementC 4 --elementD 4 --timeout 1 --smart-generation "association_needed" command has been running on my computer for about 20 minutes now. Am I calling it wrong?

I fixed this, too, I think by breaking out of the loop before any solving is started in case the timeout is already reached. Currently, the timeout also does not affect the smart generation. I will think about making it more general.

@nrueh One more question, as I am currently testing it further. Should the timeout apply to grounding as well? Because currently even in the new version, it does not seem to affect grounding: python ooasp/run.py --elementA 20 --elementB 20 --elementC 20 --elementD 20 --timeout 1 does not seem to terminate on timeout and gives this result (which seems okay if timeout should not apply to grounding):

{
    "timeout": true,
    "#objects": 80,
    "#objects_added_per_type": {
        "elementA": 20,
        "elementB": 20,
        "elementC": 20,
        "elementD": 20
    },
    "#associations": 0,
    "times": {
        "runtime": 41.895,
        "initialization": 41.895,
        "smart_generation": {
            "time": 0.0,
            "cautious": 0,
            "brave": 0,
            "functions": {}
        },
        "solve": 0,
        "ground": 41.891
    },
    "model": null
}

Currently it does not affect grounding, but in my opinion it should be applied globally (so grounding and everything else). Tt was tricky implementing it like that. But we are thinking about ways to improve it. In case you have any ideas, go ahead :)

Also we would like to make it possible to kill it from some external tool (e.g. runsolver) and still get the stats but this was also not easy...

Kriplingo commented 3 weeks ago

@susuhahnml @nrueh also I don't think the timeouts are working properly, either I am calling it wrong, or there is some mistake when it comes to terminating the solving early. python ooasp/run.py --elementA 4 --elementB 4 --elementC 4 --elementD 4 --timeout 1 --smart-generation "association_needed" command has been running on my computer for about 20 minutes now. Am I calling it wrong?

I fixed this, too, I think by breaking out of the loop before any solving is started in case the timeout is already reached. Currently, the timeout also does not affect the smart generation. I will think about making it more general.

@nrueh One more question, as I am currently testing it further. Should the timeout apply to grounding as well? Because currently even in the new version, it does not seem to affect grounding: python ooasp/run.py --elementA 20 --elementB 20 --elementC 20 --elementD 20 --timeout 1 does not seem to terminate on timeout and gives this result (which seems okay if timeout should not apply to grounding):

{
    "timeout": true,
    "#objects": 80,
    "#objects_added_per_type": {
        "elementA": 20,
        "elementB": 20,
        "elementC": 20,
        "elementD": 20
    },
    "#associations": 0,
    "times": {
        "runtime": 41.895,
        "initialization": 41.895,
        "smart_generation": {
            "time": 0.0,
            "cautious": 0,
            "brave": 0,
            "functions": {}
        },
        "solve": 0,
        "ground": 41.891
    },
    "model": null
}

Currently it does not affect grounding, but in my opinion it should be applied globally (so grounding and everything else). Tt was tricky implementing it like that. But we are thinking about ways to improve it. In case you have any ideas, go ahead :)

Also we would like to make it possible to kill it from some external tool (e.g. runsolver) and still get the stats but this was also not easy...

Yes, makes sense, python is not a nice tool to work with when it comes to timeouts, signals and multi-threading in general. I will try to think a bit on it, but currently I don't really know if there is some other way to make sure we get the output even if the process is forcefully killed, other than maybe keeping track of the state periodically and saving it in a variable which would be printed out on killing of the process, or saved in a file, but I am quite sure this would come at a cost of performance. (Not to mention the fact that I am not sure if this would even be possible with the clingo API control).

nrueh commented 3 weeks ago

Sorry, I accidentally pushed something I was trying out with heuristics in this branch but it is not supposed to go in here...I didn't have time to fix it now but I will revert it tomorrow!

nrueh commented 3 weeks ago

We could also not care about the output for benchmarking...in the end we are interested in seeing what options work and what kind of instances we can solve, right?

nrueh commented 3 weeks ago

Sorry, I accidentally pushed something I was trying out with heuristics in this branch but it is not supposed to go in here...I didn't have time to fix it now but I will revert it tomorrow!

This is fixed now.

rtaupe commented 3 weeks ago

@Kriplingo Thank you for your in-depth reviewing and commenting! :-)

rtaupe commented 3 weeks ago

We could also not care about the output for benchmarking...in the end we are interested in seeing what options work and what kind of instances we can solve, right?

I care about the stats if solving terminates. If it doesn't, I think they are less important.

nrueh commented 2 weeks ago

Code works on my side, I just need to know one more thing. Did you remove the option for choosing a number of threads (or renamed it)? So that I know if I need to take the number of threads into account for the benchmarks?

Yes, but now you can use all normal clingo options on the command line when running the program. The corresponding option in clingo is --parallel-mode or just -t

So for running with 3 threads for example:

python ooasp/app.py --frame 5 --parallel-mode=3
nrueh commented 2 weeks ago

We also removed the timeout option. If I understood well you were enforcing this with some external tool, anyways (which I think is the best way to go). However, in case you need it clingo also has a built-in timeout option --time-limit

Kriplingo commented 2 weeks ago

We also removed the timeout option. If I understood well you were enforcing this with some external tool, anyways (which I think is the best way to go). However, in case you need it clingo also has a built-in timeout option --time-limit

Yes, I use an external tool to enforce it, but just out of curiosity, since it is in-built by clingo, does this apply to grounding as well? Also thank you for all the changes, they have been extremely helpful.

nrueh commented 2 weeks ago

We also removed the timeout option. If I understood well you were enforcing this with some external tool, anyways (which I think is the best way to go). However, in case you need it clingo also has a built-in timeout option --time-limit

Yes, I use an external tool to enforce it, but just out of curiosity, since it is in-built by clingo, does this apply to grounding as well? Also thank you for all the changes, they have been extremely helpful.

Yes, it applies to everything we are doing inside the Application class. So that also involves grounding, adding objects, (cautious/brave) solving, etc...