Changes for the acrobot hackathon

[kegl]

• Optional credit tracking, set by the "TRACK_CREDITS" config variable. Credit tracking is needed if we want to compute historical contributivities. PR in ADV Add optional contributivity calculations #449
• CLI command ramp-database compute-contributivity --event --ramp-kit-dir --ramp-data-dir. It computes contributivity and historical contributivity scores (displayed in two leaderboard columns) and the combined score (displayed at the top of the leaderboard pages). It is not run automatically as before, admins need to update it time to time. Running time is quadratic in number of submissions, and can be quite long of the data sets are big (more precisely: if the combination function is slow). PR in ADV Add optional contributivity calculations #449
• CLI command ramp database make-admin --user to make existing user site admin. Added in [MRG] EHN create function to create RAMP admin #308
• Accelerating leaderboard computation when predictions are big.
• When multiple submissions are processed in the dispatcher.update_database_results, we only update the leaderboard once (not for each processed submission). Fixed in ENH dispatcher only update leaderboard once when collecting results #316
• ramp_database.tools.submission.get_scores and get_time accelerated by avoiding loading predictions from SubmissionOnCVFold. (related ordering bug fixed in BUG sort cv folds ordered during queries #321) Solved by [WIP] BUG defer loading of the predictions when not required #315
• Fixing the leaderboard train time (it displayed validation time) Solved Discrepancy between the train time and evaluation time #193 (likely in MAINT/TST/DOC: backend/frontend refactoring #117))
• beautifying the leaderboards (new order in private, rounding integers, renaming some columns)
• Toggling submissions in and out of the competition added back. The frontend button was kept but there was no view function catching it. It didn't crash, just didn't do anything. Solved in ENH toggle submission in/out of competition #351
• In the collaborative phase participants look at each other's code. There was a crash since a query tried to find the submission using the current user's name, not the submitter's name. Hotfix is done by returning True if the event is public, before querying the submission. Not yet satisfactory though, minor bug remains in code. Fixed in BUG couple of fixes regarding the frontend #334

[codecov]

Codecov Report

Merging #276 into master will decrease coverage by 0.3%.
The diff coverage is 97.37%.

@@            Coverage Diff             @@
##           master     #276      +/-   ##
==========================================
- Coverage   92.13%   91.83%   -0.31%     
==========================================
  Files          95       96       +1     
  Lines        7244     7455     +211     
==========================================
+ Hits         6674     6846     +172     
- Misses        570      609      +39

Impacted Files	Coverage Δ
...atabase/ramp_database/model/tests/test_workflow.py	100% <ø> (ø)	⬆️
ramp-utils/ramp_utils/tests/test_config_parser.py	100% <ø> (ø)	⬆️
ramp-database/ramp_database/tests/test_utils.py	100% <ø> (ø)	⬆️
ramp-frontend/ramp_frontend/tests/test_testing.py	100% <ø> (ø)	⬆️
ramp-utils/ramp_utils/tests/test_deploy.py	92.53% <ø> (ø)	⬆️
ramp-utils/ramp_utils/tests/test_cli.py	100% <ø> (ø)	⬆️
ramp-engine/ramp_engine/tests/test_dispatcher.py	100% <ø> (ø)	⬆️
...database/ramp_database/model/tests/test_problem.py	100% <ø> (ø)	⬆️
...p-database/ramp_database/model/tests/test_score.py	100% <ø> (ø)	⬆️
...mp-database/ramp_database/model/tests/test_fold.py	100% <ø> (ø)	⬆️
... and 48 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0515e92...049637b. Read the comment docs.

[kegl]

@glemaitre: does the CLI part of the sphynx doc get updated automatically as we add CLI commands or it has to be added manually to the doc?

[pep8speaks]

Hello @kegl! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-01-14 16:58:58 UTC

[kegl]

I prefer not to for readability.

[kegl]

@agramfort, @glemaitre I prefer to elaborate to avoid misunderstandings although it's a minor issue.

The new version is about 30 lines longer, 20 of which is docstring and comments. We could add helper functions to get the scores and the time and call it from both the private and public leaderboard code. That would be the proper way to modularize, but that would add code (new functions, docstrings, etc). I'm not against but I find this current compromise (between code repetition and length) better.

The previous code somehow assumed that there are only minor differences between public and private leaderboard and tried to construct them in the same function. The timing bug was introduced by an over-generalizing idea to make it "pretend" that they are more similar than they actually are. Even tests didn't catch this bug. It was quite important since in our challenge there is limit on total training time, so we needed a hotfix in the weekend. When doing it I had a hard time understanding the code, quite a bit of pandas magic. I prefer to construct the columns explicitly, for readability. In this way we can also modify the the two tables relatively independently.

[kegl]

I would like to merge this into master after the hackathon ends, so early December.

Hopefully no more hotfixes, we'll see.

We have one more customization change pending: we added a long privacy statement and terms and conditions on the sign up page (required by our lawyers). We simply modified the sign up template, but it's pure html that could be programmatically inserted from a file into the template. This should be customizable. @agramfort @glemaitre @maikia @lucyleeow what would you suggest to do here? We can have custom html files in https://github.com/paris-saclay-cds/ramp-board/tree/master/ramp-frontend/ramp_frontend/templates but is this a good practice to modify those directly (since they are part of the software, committed in the repo)?

A similar issue comes up when you want to add OVH logo to the landing page, but only on the CDS deployment.

[lgtm-com]

This pull request introduces 4 alerts when merging 91522cd into 1ba84e2 - view on LGTM.com

new alerts:

• 4 for Unused import

[rth]

I think it would help a lot reviewers if this PR was split in smaller chunks. I have not reviewed everything in details, but following parts should be very easy to merge if they were made as separate PRs and looks like a significant improvement,

• optimization in ramp-database/ramp_database/tools/frontend.py and in dispatcher ramp-engine/ramp_engine/dispatcher.py
• fixing typos in ramp_frontend/views/admin.py and ramp_frontend/views/auth.py
• fix bug in send_mail(admin: I have actually run into this previously, but hasn't opened an issue so far.
• the addition of make_admin CLI command

and in larger changes (that I have not reviewed in detail),

• adding the computation of contributivity
• and other CV changes discussed in the description

Is anyone interested/available in making smaller PRs for the top 4 points, at least? That would also help reducing the diff here. Maybe @maikia ? :)

[lgtm-com]

This pull request introduces 1 alert when merging 8e7cbe1 into c8f6423 - view on LGTM.com

new alerts:

• 1 for Unused import

[lgtm-com]

This pull request introduces 1 alert when merging 3a733e9 into c8f6423 - view on LGTM.com

new alerts:

• 1 for Unused import

[rth]

Closing as I believe all major features from this PR were either merged as separate PRs to master or are now in the advanced branch. Full list in the issue description #276 (comment)
There might be a few style related points remaining but for those it would be easier to create a new PR than trying to resolve merge conflicts here IMO.