Skip to content

Conversation

@johnbrvc
Copy link
Collaborator

Description of what the PR does

Allow "score" scoreboard type in addition to the standard "pass-fail". Save the scoreboard type in the contest's ContestInformation.
Update CLICS contest API endpoint to use the scoreboard type defined in ContestInformation instead of hard-coding pass-fail.
CI: ContestInformation was missing compares for many members in the isSameAs() method, so if any of these were different, isSameAs() would not detect that.
CI: Add string constants for contest scoreboard type.

Issue which the PR addresses

Partial fix for #1006

Environment in which the PR was developed (OS,IDE, Java version, etc.)

Windows 11

Precise steps for testing the PR (i.e., how to demonstrate that it works correctly)

Start fresh (no profile or profiles.properties)
Start a server loading the samps/pointscoring contest
It should load correctly (note that is uses scoreboard_type: score)

Allow "score" scoreboard type in addition to the standard "pass-fail".   Save the scoreboard type in the contest's ContestInformation.
Update CLICS contest API endpoint to use the scoreboard type defined in ContestInformation instead of hard-coding pass-fail.
CI: ContestInformation was missing compares for many members in the isSameAs() method, so if any of these were different, isSameAs() would not detect that.
CI: Add string constants for contest scoreboard type.
@johnbrvc johnbrvc requested a review from clevengr July 11, 2025 12:24
@johnbrvc johnbrvc self-assigned this Jul 11, 2025
@johnbrvc johnbrvc added the One Approval A very low risk and low LOE PR/Change label Jul 11, 2025
johnbrvc added 3 commits July 11, 2025 08:39
Some JUnit tests read a CDP that contains problem datafiles.  These JUnits now make use of the newer TestDataGroups() class to read test data as a result of using ContestSnakeYamlLoader.  This class requires a valid Log object.  ContestSnakeYamlLoader uses StaticLog(), as a result, the static log must have been previously intialized.  AbstractTestCase.ensureStaticLog() does this.
Changes to DSA to support both pass-fail and point scoring contests.
Update CLICS API to deal with scoring contests and only put out scoring related properties if it is a point scoring contest.
Copy link
Contributor

@clevengr clevengr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reviewed all but one of the (20) changed files in this PR (the changes to DefaultScoringAlgorithm are extensive and I need more time to look at those).

I noted once place where I think there is a logic error (in DefaultPointScoringStandingsRecordComparator.java). Other than that I made only minor comments, which I'd like to see considered and addressed, but wouldn't stop me from Approving the PR.

Given that there's no practical way to TEST this PR until the entire Point-Scoring pipeline implementation is done, I would simply approve the PR once the comments I've made are addressed.

johnbrvc added 2 commits July 16, 2025 09:40
Fix spelling errors in comments.
Fix wrong types in comments.
It's not intuitively obvious how the comparator arrives at its return result.  Hopefully the comment added explains it.
Copy link
Contributor

@clevengr clevengr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have re-reviewed all changed files, and also completed a cursory review of DefaultScoringAlgorithm. I'm not sure about all the changes in DefaultScoringAlgorithm; they look reasonable but there are a lot of details in the changes. That said, and given that there's no way to do a run-time test of the PR, I'm approving the PR so we can move forward.

Comment on lines +306 to +308
if (!solved) {
score = 0.0;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be making a hard-coded decision that if the problem is not solved then the score is 0.0. It seems like there could be situations where the Grader could have assigned "some points" (for example, due to having passed "some tests" but not all) even if the problem wasn't "entirely solved". Shouldn't the score assignment here be coming from some input somewhere (like, the JudgementRecord) rather than being hard-coded to be zero?

johnbrvc added 2 commits July 18, 2025 09:27
Use Double object instead of primative.  This way we don't have to filter and can just use the "only include if non-null" annotation instead of all the extra rtfilter code.
Fix copyright date in ProblemDataFiles.
If using the LegacyGrader internally (and not as an external process), the "String gradeTestCases(ArrayList<String> testCaseResults)" may be used directly.  The bypasses the extra steps of saving the list of judgment/score tuples to a file and having the grader open and read that file.  In addition, the gradTestCases method returns the net judgment string, eg. "AC 50" on success, and null on failure.
@johnbrvc johnbrvc added this to the 9.11.0 milestone Jul 26, 2025
johnbrvc added 2 commits July 27, 2025 08:02
Specification illudes to how to implement ignore_sample.  It relies on the fact that it may be used only at the root level and there must be 2 groups (sample and secret) at the root level. *sigh*  What a hacky way to implement it, but - that's the spec.
Add JUnits to test the flag - 3 cases.
validCDP used to form the paths to judge's data on the assumption thta everything was in either sample or secret.  With test data groups, that is no longer the case, so change validateCDP to use the files from the ProblemDataFiles, which, is probably what it should have done in the first place.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

One Approval A very low risk and low LOE PR/Change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants