JOJ/JOJ3-config-generator
27
1
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(diff): bugs on diff stdin and numerics #16

Merged
张泊明518370910136 merged 22 commits from fix/diff into master 2025-05-24 02:45:39 +08:00
Conversation 1 Commits 22 Files Changed 9 +15 -4
3 changed files with 15 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 183e6f1545 - Show all commits

2
joj3_config_generator/models/const.py
View File

@ -6,6 +6,8 @@ DEFAULT_CPU_LIMIT = Time("1s")
DEFAULT_MEMORY_LIMIT = Memory("256m")
DEFAULT_FILE_LIMIT = Memory("32m")
DEFAULT_CASE_SCORE = 5
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-05-22 15:12:26 +08:00
Outdated
Review
Copy Link

why is it removed?

why is it removed?
李衍志523370910113 commented 2025-05-22 15:17:04 +08:00
Outdated
Review
Copy Link

added a field in toml named diff.default_score and this 5 is now directly written numerically here:

class ParserDiff(BaseModel):
    output: Outputs = Outputs()
    default_score: int = 5

added back now.

added a field in toml named `diff.default_score` and this 5 is now directly written numerically here: ``` class ParserDiff(BaseModel): output: Outputs = Outputs() default_score: int = 5 ``` --- added back now.
张泊明518370910136 commented 2025-05-22 15:18:27 +08:00
Outdated
Review
Copy Link

why not

class ParserDiff(BaseModel):
    output: Outputs = Outputs()
    default_score: int = DEFAULT_CASE_SCORE
why not ``` class ParserDiff(BaseModel): output: Outputs = Outputs() default_score: int = DEFAULT_CASE_SCORE ```
李衍志523370910113 commented 2025-05-22 15:19:51 +08:00
Review
Copy Link

yes, this is the case now, sorry :)

yes, this is the case now, sorry :)
李衍志523370910113 commented 2025-05-22 15:20:25 +08:00
Review
Copy Link

do we need to also create a DEFAULT_PROC_LIMIT?

do we need to also create a `DEFAULT_PROC_LIMIT`?
张泊明518370910136 commented 2025-05-22 15:21:49 +08:00
Review
Copy Link

good idea, maybe we can also have DEFAULT_CLOCK_LIMIT_MULTIPLIER

good idea, maybe we can also have `DEFAULT_CLOCK_LIMIT_MULTIPLIER`
李衍志523370910113 commented 2025-05-22 15:28:10 +08:00
Outdated
Review
Copy Link

both added now.

both added now.
DEFAULT_CLOCK_LIMIT_MULTIPLIER = 2
DEFAULT_PROC_LIMIT = 50
JOJ3_CONFIG_ROOT = Path("/home/tt/.config/joj")
TEAPOT_CONFIG_ROOT = Path("/home/tt/.config/teapot")

9
joj3_config_generator/models/result.py
View File

@ -3,9 +3,11 @@ from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union
from pydantic import BaseModel, ConfigDict, Field, field_validator
from joj3_config_generator.models.const import (
DEFAULT_CLOCK_LIMIT_MULTIPLIER,
bomingzh marked this conversation as resolved
张泊明518370910136 commented 2025-05-23 07:45:05 +08:00
Review
Copy Link

Should be applied to other locations in transformers/task.py

Should be applied to other locations in `transformers/task.py`
DEFAULT_CPU_LIMIT,
DEFAULT_FILE_LIMIT,
DEFAULT_MEMORY_LIMIT,
DEFAULT_PROC_LIMIT,
)
@ -49,10 +51,13 @@ class Cmd(BaseModel):
stdout: Union[Collector, StreamOut] = Collector(name="stdout")
stderr: Union[Collector, StreamOut] = Collector(name="stderr")
cpu_limit: int = Field(DEFAULT_CPU_LIMIT, serialization_alias="cpuLimit")
clock_limit: int = Field(2 * DEFAULT_CPU_LIMIT, serialization_alias="clockLimit")
clock_limit: int = Field(
DEFAULT_CLOCK_LIMIT_MULTIPLIER * DEFAULT_CPU_LIMIT,
serialization_alias="clockLimit",
)
memory_limit: int = Field(DEFAULT_MEMORY_LIMIT, serialization_alias="memoryLimit")
stack_limit: int = Field(0, serialization_alias="stackLimit")
proc_limit: int = Field(50, serialization_alias="procLimit")
proc_limit: int = Field(DEFAULT_PROC_LIMIT, serialization_alias="procLimit")
cpu_rate_limit: int = Field(0, serialization_alias="cpuRateLimit")
cpu_set_limit: str = Field("", serialization_alias="cpuSetLimit")
copy_in: Dict[str, InputFile] = Field({}, serialization_alias="copyIn")

8
joj3_config_generator/transformers/task.py
View File

@ -203,11 +203,14 @@ def fix_diff(
# duplicate with the fallback case in executor.with_
if cmd.cpu_limit == const.DEFAULT_CPU_LIMIT:
bomingzh marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-05-22 16:06:21 +08:00
Outdated
Review
Copy Link

What if the with_.default.cpu_limit is not the same as DEFAULT_CPU_LIMIT? Why do we need to set these fields to none?

What if the `with_.default.cpu_limit` is not the same as `DEFAULT_CPU_LIMIT`? Why do we need to set these fields to none?
李衍志523370910113 commented 2025-05-22 17:20:27 +08:00
Outdated
Review
Copy Link

if the with_.default.cpu_limit is not the same as DEFAULT_CPU_LIMIT it means its already been input before, and it is considered as the new default value for all cases (ta might want to control it). If I dont set these field to none, it will use DEFAULT_CPU_LIMIT instead of those ta input, which is not intended. It solve the second problem in #15

if the `with_.default.cpu_limit` is not the same as `DEFAULT_CPU_LIMIT` it means its already been input before, and it is considered as the new default value for all cases (ta might want to control it). If I dont set these field to none, it will use `DEFAULT_CPU_LIMIT` instead of those ta input, which is not intended. It solve the second problem in https://focs.ji.sjtu.edu.cn/git/JOJ/JOJ3-config-generator/issues/15
张泊明518370910136 commented 2025-05-23 07:44:02 +08:00
Outdated
Review
Copy Link

Which test case will show this problem?

Which test case will show this problem?
张泊明518370910136 commented 2025-05-23 08:07:15 +08:00
Outdated
Review
Copy Link

We need another pydantic model for auto detected cases. Fields in these cases can be none, which means they are not set and should use with_.default values.

We need another pydantic model for auto detected cases. Fields in these cases can be none, which means they are not set and should use `with_.default` values.
cmd.cpu_limit = None
if cmd.clock_limit == 2 * const.DEFAULT_CPU_LIMIT:
if (
cmd.clock_limit
== const.DEFAULT_CLOCK_LIMIT_MULTIPLIER * const.DEFAULT_CPU_LIMIT
):
cmd.clock_limit = None
if cmd.memory_limit == const.DEFAULT_MEMORY_LIMIT:
cmd.memory_limit = None
if cmd.proc_limit == executor.with_.default.proc_limit:
if cmd.proc_limit == const.DEFAULT_PROC_LIMIT:
cmd.proc_limit = None
stage_cases.append(cmd)
parser_case = result.DiffCasesConfig(
jon-lee marked this conversation as resolved
张泊明518370910136 commented 2025-05-23 08:10:29 +08:00
Review
Copy Link

better check if the *.out file exists in get_testcases

better check if the `*.out` file exists in get_testcases
李衍志523370910113 commented 2025-05-23 17:01:18 +08:00
Review
Copy Link

Not quite understand why would taht better?

Not quite understand why would taht better?
张泊明518370910136 commented 2025-05-23 17:06:10 +08:00
Review
Copy Link

If case*.out will always be used in diff parser, we want to ensure it exists to form a valid case.

If case*.out will always be used in diff parser, we want to ensure it exists to form a valid case.
李衍志523370910113 commented 2025-05-23 19:27:29 +08:00
Review
Copy Link

ok, I see, indeed a good point.

ok, I see, indeed a good point.
李衍志523370910113 commented 2025-05-23 20:26:47 +08:00
Review
Copy Link

done.

done.
@ -229,6 +232,7 @@ def fix_diff(
cpu_limit=None,
clock_limit=None,
memory_limit=None,
proc_limit=None,
)
stage_cases.append(cmd)
parser_case = result.DiffCasesConfig(