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

fix problems in #15

fix problems in https://focs.ji.sjtu.edu.cn/git/JOJ/JOJ3-config-generator/issues/15
李衍志523370910113 added 14 commits 2025-05-22 15:01:37 +08:00
feat(diff): add auto detect testcases feature
Some checks failed
build / build (push) Failing after 1m38s
e4246ecf45
fix(diff): use list instead of set to past tests
Some checks failed
build / build (push) Failing after 1m38s
890120c479
fix: master conflict
Some checks failed
build / build (push) Failing after 1m41s
45903ae308
fix: pytest
Some checks failed
build / build (push) Failing after 1m42s
a4717268ec
fix: sort default cases
Some checks failed
build / build (push) Failing after 3m22s
e775992e5d
Merge branch 'master' into fix/diff
All checks were successful
build / build (push) Successful in 1m48s
ba456cf7f0
feat: support for subfolder change param
All checks were successful
build / build (push) Successful in 1m50s
build / build (pull_request) Successful in 1m37s
a8a7fd47a0
style: use const
All checks were successful
build / build (push) Successful in 1m53s
build / build (pull_request) Successful in 1m53s
391d61d006
fix(diff): cases not properly detected
Some checks failed
build / build (push) Has been cancelled
26df677b36
fix: merge conflict
All checks were successful
build / build (push) Successful in 1m24s
2c3944ea3f
test: case.command
All checks were successful
build / build (push) Successful in 1m40s
build / build (pull_request) Successful in 1m44s
2f1686c1e3
fix: remove stdin for diff stage
All checks were successful
build / build (push) Successful in 2m1s
5acb2ca201
feat: add default score for diff
All checks were successful
build / build (push) Successful in 1m54s
ca63a94238
fix: cases specific numerics remove when unecessary
All checks were successful
build / build (push) Successful in 1m43s
build / build (pull_request) Successful in 1m22s
927e5ce95d
张泊明518370910136 was assigned by 李衍志523370910113 2025-05-22 15:02:16 +08:00
李衍志523370910113 added 1 commit 2025-05-22 15:03:37 +08:00
fix: conflict files
All checks were successful
build / build (pull_request) Successful in 1m48s
build / build (push) Successful in 1m45s
68fedcfe95
张泊明518370910136 reviewed 2025-05-22 15:12:26 +08:00
@ -5,7 +5,6 @@ from joj3_config_generator.models.common import Memory, Time
DEFAULT_CPU_LIMIT = Time("1s")
DEFAULT_MEMORY_LIMIT = Memory("256m")
DEFAULT_FILE_LIMIT = Memory("32m")
DEFAULT_CASE_SCORE = 5

why is it removed?

why is it removed?
Author
Member

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.

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 ```
Author
Member

yes, this is the case now, sorry :)

yes, this is the case now, sorry :)
Author
Member

do we need to also create a DEFAULT_PROC_LIMIT?

do we need to also create a `DEFAULT_PROC_LIMIT`?

good idea, maybe we can also have DEFAULT_CLOCK_LIMIT_MULTIPLIER

good idea, maybe we can also have `DEFAULT_CLOCK_LIMIT_MULTIPLIER`
Author
Member

both added now.

both added now.
jon-lee marked this conversation as resolved
李衍志523370910113 added 1 commit 2025-05-22 15:18:53 +08:00
fix: add back DEFAULT_CASE_SCORE
All checks were successful
build / build (pull_request) Successful in 1m33s
build / build (push) Successful in 1m42s
b4a9d0cb62
李衍志523370910113 added 1 commit 2025-05-22 15:27:52 +08:00
feat: add DEFAULT_PROC_LIMIT n DEFAULT_CLOCK_LIMIT_MULTIPLIER
All checks were successful
build / build (push) Successful in 1m32s
build / build (pull_request) Successful in 1m40s
183e6f1545
张泊明518370910136 reviewed 2025-05-22 16:03:39 +08:00
@ -238,6 +246,7 @@ def fix_diff(
)
parser_cases.append(parser_case)
executor.with_.cases = stage_cases
executor.with_.default.stdin = None

Should be removed. It should be fixed from JOJ3 side. Also in InputFile = Union[LocalFile, MemoryFile, PreparedFile, Symlink, None]

Should be removed. It should be fixed from JOJ3 side. Also in `InputFile = Union[LocalFile, MemoryFile, PreparedFile, Symlink, None]`
Author
Member

done

done
jon-lee marked this conversation as resolved
李衍志523370910113 added 1 commit 2025-05-22 16:06:13 +08:00
fix: remove none stdin for diff
All checks were successful
build / build (push) Successful in 13m50s
build / build (pull_request) Successful in 14m0s
dcebf86408
张泊明518370910136 reviewed 2025-05-22 16:06:21 +08:00
@ -202,2 +202,3 @@
cmd.args = None
if cmd.cpu_limit == executor.with_.default.cpu_limit:
# duplicate with the fallback case in executor.with_
if cmd.cpu_limit == const.DEFAULT_CPU_LIMIT:

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?
Author
Member

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

Which test case will show this problem?

Which test case will show this problem?

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.
bomingzh marked this conversation as resolved
张泊明518370910136 reviewed 2025-05-23 07:45:05 +08:00
@ -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,

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

Should be applied to other locations in `transformers/task.py`
bomingzh marked this conversation as resolved
张泊明518370910136 reviewed 2025-05-23 08:10:29 +08:00
@ -210,3 +213,4 @@
if cmd.proc_limit == const.DEFAULT_PROC_LIMIT:
cmd.proc_limit = None
stage_cases.append(cmd)
parser_case = result.DiffCasesConfig(

better check if the *.out file exists in get_testcases

better check if the `*.out` file exists in get_testcases
Author
Member

Not quite understand why would taht better?

Not quite understand why would taht better?

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.
Author
Member

ok, I see, indeed a good point.

ok, I see, indeed a good point.
Author
Member

done.

done.
jon-lee marked this conversation as resolved
张泊明518370910136 added 1 commit 2025-05-23 08:46:14 +08:00
fix: default limit
All checks were successful
build / build (push) Successful in 1m42s
build / build (pull_request) Successful in 2m2s
c3f2b21732

Please check if the current solution works.

Please check if the current solution works.
李衍志523370910113 added 1 commit 2025-05-23 20:26:41 +08:00
feat: add verification for path *.out
All checks were successful
build / build (pull_request) Successful in 1m33s
build / build (push) Successful in 1m48s
2dbfa986fa
张泊明518370910136 reviewed 2025-05-23 20:30:58 +08:00
@ -251,4 +260,7 @@ def get_testcases(
testcases_path.relative_to((task_root / task_path).parent)
).removesuffix(".in")
)
assert os.path.exists(

No. Just do not append it to the return value and log a warning.

No. Just do not append it to the return value and log a warning.
Author
Member

okay

okay
Author
Member

we can probably move some redundant functions like get_testcaes into a utils.py as file llines already bit large

we can probably move some redundant functions like `get_testcaes` into a `utils.py` as file llines already bit large

leave it here now. we have not reused the logic.

leave it here now. we have not reused the logic.
jon-lee marked this conversation as resolved
李衍志523370910113 added 1 commit 2025-05-23 20:39:16 +08:00
fix: use logger instead of direct assert
All checks were successful
build / build (push) Successful in 1m39s
build / build (pull_request) Successful in 1m50s
7265411abe
张泊明518370910136 reviewed 2025-05-23 20:42:10 +08:00
@ -246,6 +256,9 @@ def get_testcases(
) -> Set[str]: # basedir here should be task_conf.root / task_conf.path
testcases = set()
for testcases_path in (task_root / task_path).parent.glob("**/*.in"):
if not os.path.exists(str(testcases_path).removesuffix(".in") + ".out"):

if not testcases_path.with_suffix(".out").exists():

`if not testcases_path.with_suffix(".out").exists():`
Author
Member

done

done
jon-lee marked this conversation as resolved
李衍志523370910113 added 1 commit 2025-05-23 20:43:25 +08:00
fix: use logger instead of direct assert
All checks were successful
build / build (pull_request) Successful in 1m50s
build / build (push) Successful in 1m52s
044d0f0d41
张泊明518370910136 approved these changes 2025-05-24 02:44:19 +08:00
张泊明518370910136 merged commit b1ea7d9591 into master 2025-05-24 02:45:39 +08:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: JOJ/JOJ3-config-generator#16
No description provided.