codalab / codabench

Codabench is a flexible, easy-to-use and reproducible benchmarking platform. Check our paper at Patterns Cell Press https://hubs.li/Q01fwRWB0
Apache License 2.0
76 stars 28 forks source link

Optimization `PR#2` - Submissions and Participants Count #1669

Open ihsaan-ullah opened 1 week ago

ihsaan-ullah commented 1 week ago

@ mention of reviewers

@Didayolo

A brief description of the purpose of the changes contained in this PR.

This is a first PR in the optimization effort to load codabench quickly. The following are done in this PR:

  1. Adde new fields to competition modal participants_count and submissions_count
  2. Used these fields in the competition detail page
  3. Used participants_count field in public competitions page
  4. Used participants_count field in the popular competitions section in home page

On the backend, serializers are changed to use this new filed instead of the annotated values from the queries.

NOTE

  1. when you upload a new submission, submissions_count will be updated (+1)
  2. When you delete a submission, submissions_count will be updated (-1)
  3. When a new participant joins the competition (even not approved yet) the participants_count will update (+1)
  4. participants_count is never decremented because we do not delete a participant we only revoke the status

Manual update

docker compose exec django ./manage.py migrate

NEXT

We need a script that will update all the competitions with the right submissions and participants counts

Issues this PR resolves

Checklist

Didayolo commented 1 week ago

Very nice work. I'll review it asap.

Didayolo commented 1 week ago

@ihsaan-ullah Any idea why your change triggers the following error?

=================================== FAILURES ===================================
____________ SubmissionMixinTests.test_invalid_submission_not_sent _____________

self = <chahub.tests.test_chahub_mixin.SubmissionMixinTests testMethod=test_invalid_submission_not_sent>

    def test_invalid_submission_not_sent(self):
        self.submission.status = "Running"
        self.submission.is_public = False
        resp1 = self.mock_chahub_save(self.submission)
>       assert not resp1.called
E       AssertionError: assert not True
E        +  where True = <MagicMock name='_send' id='139945117507488'>.called

src/apps/chahub/tests/test_chahub_mixin.py:49: AssertionError
----------------------------- Captured stderr call -----------------------------
ChaHub :: Submission(224) is_valid = False
ChaHub :: Competition(161) is_valid = True
ChaHub :: Received response 201 
Task chahub.tasks.send_to_chahub[2ffe7de6-212d-47c6-b4c1-fc6971c5ab19] succeeded in 0.004535872000019481s: None
------------------------------ Captured log call -------------------------------
INFO     chahub.models:models.py:122 ChaHub :: Submission(224) is_valid = False
INFO     chahub.models:models.py:122 ChaHub :: Competition(161) is_valid = True
INFO     chahub.tasks:tasks.py:56 ChaHub :: Received response 201 
INFO     celery.app.trace:trace.py:125 Task chahub.tasks.send_to_chahub[2ffe7de6-212d-47c6-b4c1-fc6971c5ab19] succeeded in 0.004535872000019481s: None
____ SubmissionMixinTests.test_retrying_invalid_submission_wont_retry_again ____

self = <chahub.tests.test_chahub_mixin.SubmissionMixinTests testMethod=test_retrying_invalid_submission_wont_retry_again>

    def test_retrying_invalid_submission_wont_retry_again(self):
        self.submission.status = "Running"
        self.submission.chahub_needs_retry = True
        resp = self.mock_chahub_save(self.submission)
>       assert not resp.called
E       AssertionError: assert not True
E        +  where True = <MagicMock name='_send' id='139945080014256'>.called

src/apps/chahub/tests/test_chahub_mixin.py:59: AssertionError
ihsaan-ullah commented 6 days ago

I checked the tests but could not figure out why they are failing. I will spend some more time on this soon

ihsaan-ullah commented 5 days ago

@Didayolo I think these tests are useless because they are suppose to send submissions to ChaHub but there is no ChaHub right?

Didayolo commented 5 days ago

@Didayolo I think these tests are useless because they are supposed to send submissions to ChaHub but there is not ChaHub right?

@ihsaan-ullah That is a good remark, however we may want to understand why these tests started to fail with this PR specifically.

Didayolo commented 3 days ago

Maybe the problem is occurs because https://chahub.org/ does get the number of participants of the competitions? The renaming of participants_count may lead to a bug.

ihsaan-ullah commented 3 days ago

Maybe the problem is occurs because https://chahub.org/ does get the number of participants of the competitions? The renaming of participants_count may lead to a bug.

The problem is in sending submissions to chahub but in the test there is a dummy chahub (no real URL is used) so that should not be a problem

ihsaan-ullah commented 7 hours ago

The tests are failing because of these lines of code added in the Submission modal's save function

if is_new:
      # Increment the submissions_count for the competition
      self.phase.competition.submissions_count += 1
      self.phase.competition.save()

Removing this code fixes the problem. The real issue here is the test itself. If we don't have any chahub then why do we have tests for it and why do we have chahub fields in the submission?

I think we should do the following:

ihsaan-ullah commented 7 hours ago

Added tests for submisisons count and participants count