Skip to content

Warn when no CPU-time limit is specified when submitting runs to BenchCloud#1254

Open
Po-Chun-Chien wants to merge 2 commits intomainfrom
cloud-warn-no-cpu-time-limit
Open

Warn when no CPU-time limit is specified when submitting runs to BenchCloud#1254
Po-Chun-Chien wants to merge 2 commits intomainfrom
cloud-warn-no-cpu-time-limit

Conversation

@Po-Chun-Chien
Copy link
Copy Markdown
Member

No description provided.

@marian-lingsch
Copy link
Copy Markdown
Contributor

LGTM, thanks!

@marian-lingsch
Copy link
Copy Markdown
Contributor

marian-lingsch commented Feb 23, 2026

Maybe add a small time delay (one second or so) between when the warning is printed and the code continues such that it is not lost in all the other warnings about missing files (for example for validation runs)

@PhilippWendler
Copy link
Copy Markdown
Member

Maybe add a small time delay (one second or so) between when the warning is printed and the code continues such that it is not lost in all the other warnings about missing files (for example for validation runs)

No, we do not want that. If every part of the program starts to arbitrarily add sleep commands to its warnings, then this will become an inconsistent annoyance for users. There is also no meaningful value for the sleep, and it nevertheless does not guarantee that a user sees the warning. There is a reason that no command-line utility uses sleep for warnings. Either the warning is important enough that the user must act on it, then it should not be a warning but trigger an error, or the warning is not important enough, then it remains an ordinary warning.

We discussed the sleep recently in a different context where the goal was to show a specific temporary warning from a human to the user, and where "make it an error and fail execution" was not a viable alternative. This is a different situation and using sleep there does not mean it should be used elsewhere.

@PhilippWendler
Copy link
Copy Markdown
Member

Note that to be effective, this must also be implemented for the WebClient mode.

But more importantly, I am not convinced that a warning is a good idea here. A warning message should be the least resort if there is no better alternative.
What is the goal of the default limit? Are there better alternatives?

@Po-Chun-Chien
Copy link
Copy Markdown
Member Author

What is the goal of the default limit? Are there better alternatives?

I assume the purpose of the default limit is to prevent users from submitting indefinitely long runs.
Also, I think in the current implementation, a CPU-time limit is a required field for run submissions when using the benchmarking client.

For the webclient, the current script does not define a default CPU-time limit.
However, it seems that the webclient enforces a predefined limit (configured in the Webclient_Config file) if no limit is explicitly specified.

IMO, it would be better to make the CPU-time limit mandatory and abort the submission if it is missing.

@PhilippWendler
Copy link
Copy Markdown
Member

I guess this conceptual discussion would be better on the BenchCloud issue tracker.

Note that the question is also for example whether a CPU-time limit is necessary if there is a wall-time limit, and how this combines with the limited permissions that some users have (e.g., limited maximum time per run).

@PhilippWendler PhilippWendler added the BenchCloud related to the BenchCloud integration label Feb 24, 2026
@Po-Chun-Chien
Copy link
Copy Markdown
Member Author

Po-Chun-Chien commented Feb 24, 2026

I guess this conceptual discussion would be better on the BenchCloud issue tracker.

Yes, the more extensive discussion involving potential changes to BenchCloud should take place there.

But we can already decide here whether we want to log a warning or an error instead of silently assigning a default time limit that users might easily overlook.
This would already be an improvement for me.
(If this is not wanted, we can simply close the PR.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BenchCloud related to the BenchCloud integration

Development

Successfully merging this pull request may close these issues.

3 participants