Pull request: https://github.com/aibasel/downward/pull/174

Merged.

Sounds good. 18 seconds is almost twice as long as before but I guess these are configurations we care about, so it makes sense to spend time testing them.

With the FDSS 2023 configs the "tox -e driver" test takes 18s on my machine. We already detect duplicate configs across portfolios.

How long will it take with the FDSS 2023 configurations? Also: do the configurations of different portfolios overlap? If both FDSS 2011 and 2023 run, say, LAMA we could first collect all of them in a set. (Maybe you are already doing this. We discussed a set of configurations on Github and I'm not sure anymore if this was within one portfolio or across portfolios.)

If we just test the full portfolios, "tox -e driver" takes 10s, with this patch it takes 20s. Since merging issue1110 (integrating FDSS portfolios from IPC 2023) will further increase the time for this test, I changed the patch to call the search component directly, avoiding the need to start a new Python interpreter for each tested config. This decreases the time to 10s again.

Sorry, I didn't see Malte's message before writing. good to merge, then.

Did you run the tests locally once to see if there is anything that now takes much longer? If they don't take too long, I have no objections. This is just adding an additional test and not that much code, and the new code does not introduce any new dependencies.

I don't know if you want me to weigh in, but just in case: no objections from my side.

Thanks for the review!

Yes, I intentionally left the whole-portfolio tests in place.

1) Makes sens, I changed this to use the original cost functions.
2) Could be, but it's probably negligible for the gripper:prob01 task that we use for testing this.

Can this be merged?

Looks fine to me, but it looks like you are testing the individual configs in addition to the portfolio, not instead of (at least I didn't see any removed test). This is fine with me, as I think it makes sense to also test if the portfolio execution mechanism works.

Two small things to potentially worry about:

1) Are the hard-coded replacements the settings we want to test? For example, it might make sense to test with the original cost function instead.
        ("H_COST_TRANSFORM", "adapt_costs(one)"),
        ("S_COST_TYPE", "one"),
        ("BOUND", "infinity"),

2) I don't know if any of our portfolios is in this situation, but if a portfolio has a configuration with a long-running precomputation as a later (fall-back) component, this might not have been run by the tests so far (because an earlier component solved the task) and now could make the new tests very slow. I guess we'll notice this in the Github actions.

Instead of just running each portfolio as a whole and letting the optimal portfolios stop after the first successful config, I propose to test each individual config.