Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What judgement is the current judgment for a submission? #53

Open
jsannemo opened this issue Oct 4, 2021 · 11 comments
Open

What judgement is the current judgment for a submission? #53

jsannemo opened this issue Oct 4, 2021 · 11 comments

Comments

@jsannemo
Copy link

jsannemo commented Oct 4, 2021

This is not specified. What makes most sense is probably the one last created in the feed. Why not the one in the feed with latest creation time? Because the creation time in the CCS is typically when it is created internally (i.e. when a rejudgement is started), while the order in which they become the "active" one is typically when it is added to the feed (i.e. when a rejudgement is accepted in the CCS).

A corresponding update must be added to the judgement API, to specify that its output order specifies, for a submission, the order in which the judgment became current (or at least that the last judgement is the current one).

@deboer-tim
Copy link
Member

I agree this should be specified to avoid any confusion or different implementations.

I checked and this is how the CDS would behave today, but Fredrik has been pretty adamant that we should not rely on order of elements in the feed though - the justification has been that Kattis is distributed and if two judgements happen on different hosts the events may still get output in 'reverse time' order. But, if judgements are automatic and rejudgements require human accepting, maybe that's moot.

Can we get someone from DOMjudge or PC^2 to comment? To me this is mostly a question of how rejudgements work in each CCS and hence what implementation would be more natural.

@nickygerritsen
Copy link
Collaborator

DOMjudge does it different, but I think what we do is wrong. We should spec it beter indeed

@eldering
Copy link
Collaborator

I agree with Nicky that the problem is that the current specification does not really support rejudgements. When we have that, I think we should be able to support any reasonable specification.
Two possible options, just at first thought:

  • add a valid field to a judgement, with requirement that at most one judgement per submission may be valid.
  • add a reference current_judgement_id to submission that can be null.
    Each will lead to judgements or submissions being not immutable anymore (although that was already somewhat true for judgements). I think it's best to discuss this.

@nickygerritsen
Copy link
Collaborator

Note that DOMjudge (internally) already has a valid field (we even expose it in the feed, you all just ignore it). However, the drawback is that if a new judgement becomes valid, you also need to update a previous one. But adding a reference to submission means we need to update that. Personally I think the valid field sounds better, but indeed, let's discuss this at the next meeting.

@niemela
Copy link
Member

niemela commented Oct 17, 2021

I would argue that the current spec is not intended to support rejudgements (or rather multiple judgements per submission). Specifically, potential rejudgements (see also the relevant section in the CCS reqs) were not supposed to be visible.

The way to support rejudgements within the current spec would be to either update the existing judgement (which would be suitable only in the case of manual override) or to delete the existing judgement and insert a new one. If we want to double down on this path we should clarify that there can be at most 1 judgement per submission.

If we want the ability for downstream systems to see potential rejudgements as they happen, this would not work. If so, adding some kind of current flag on the judgement would make sense. The requirement would then be that there can be at most 1 current judgement per submission.

@clevengr
Copy link
Collaborator

PC2 supports multiple judgements per submission. The most recent judgement is flagged as "current" and is what is used in (most) displays (e.g. scoreboards, Admin lists, reports, etc.), but there is a function to show the entire list of judgements which have been applied to a submission (and which judge applied each judgement and when).

IMHO we should be going in the direction of supporting multiple judgements per submission, not in the direction of requiring a new submission to replace/eliminate a previous one. Rejudging doesn't happen that often -- but it DOES happen (the WF in San Antonio rejudged an entire problem -- hundreds of submissions -- after the contest ended). The history of judgments applied to a submission is potentially a useful/important piece of data; I don't like the idea of writing a spec that causes elimination of data.

@niemela
Copy link
Member

niemela commented Oct 18, 2021

I don't like the idea of writing a spec that causes elimination of data.

If we save the event feed (which we do), no data would be lost.

It's not a question of "supporting rejudgements" or not, it's a question of how this is made available to downstream clients.

@clevengr
Copy link
Collaborator

@niemela You said:

The way to support rejudgements within the current spec would be to either update the existing judgement (which would be suitable only in the case of manual override) or to delete the existing judgement and insert a new one. If we want to double down on this path we should clarify that there can be at most 1 judgement per submission.

My comment about not wanting to write a spec that causes elimination of data was in response to this. I don't think we want to mandate that a CCS can only maintain one judgement per submission, which is what I thought you were suggesting as (one) alternative.

I do agree that we need to clarify how multiple judgments should be handled....

@niemela
Copy link
Member

niemela commented Oct 18, 2021

I don't think we want to mandate that a CCS can only maintain one judgement per submission

I definitely agree. My point is that this what we are discussing is the API interface, not the internal state of the CCS.

@clevengr
Copy link
Collaborator

Ok, so what's the actually question?

Is it, "Should multiple judgements for a submission appear in the API?" If that's the question, then I don't see how the answer can be other than "Yes", because the downstream clients certainly need to know if there's a new judgement which is replacing a previous judgement.

Or, is the question "how does the downstream client know which (of several) judgements in the feed is "current"? (And yes, I realize that THIS is literally the title of this issue; the discussion however seems to have veered a bit away from this...) However, if indeed this is the question, then I'm not sure I follow @jsannemo 's argument about not wanting to use the time of the judgement as the determining factor. If the CCS stamps judgement B with a time later than judgement A, what reason would there be for presuming that A (the earlier judgement) takes precedence over B (the later judgment), regardless of what order they appear in the event feed? What am I missing?

@niemela
Copy link
Member

niemela commented Oct 18, 2021

Is it, "Should multiple judgements for a submission appear in the API?"

This is not the "root" question, but one possible answer to the root question ("how do we know which judgement is the current when if there are several") is to not allow there to be several.

If that's the question, then I don't see how the answer can be other than "Yes", because the downstream clients certainly need to know if there's a new judgement which is replacing a previous judgement.

And deleting the first judgement (in the API) and inserting a new one would let them know this, so it's certainly possible for the answer to be "No".

what reason would there be for presuming that A (the earlier judgement) takes precedence over B (the later judgment), regardless of what order they appear in the event feed? What am I missing?

The CCS:s have a (required) feature that means that rejudgements are run "in the background" and judges get to decide if the new judgement will be used at a later stage. So the normal flow for rejudgement (again, according to the spec) is to (1) rejudge a set of submissions, then (2) review the results, and then (3) either accept or reject the rejudgements. So, between (1) and (2) the earlier judgement will still be current (even though the later judgement exists at this point, and in the case where the judges rejects the rejudgement, the earlier judgement will contine to be current "forever".

So, I would agree with @jsannemo that the time can't be used for this. If we want rejudgements to be completely transparent downstream (which I'm not so sure we do?) then we pretty much have to add some current flag for the judgements. OTOH, it would not be hard to do so, I think we all already do this internally anyway?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants