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

Dynamic Course Configurable EC calculation #584

Merged
merged 10 commits into from
Oct 8, 2024
Merged

Dynamic Course Configurable EC calculation #584

merged 10 commits into from
Oct 8, 2024

Conversation

robwaz
Copy link
Member

@robwaz robwaz commented Oct 3, 2024

Note: This PR is coupled with the corresponding sensai changes which use the added API endpoints.

API changes

  • Add new API endpoint for bot to use to submit liked meme events
  • Add new API endpoint for bot to use to submit thanks events
  • Add new API endpoint for bot to get individual thanks count
  • Add new API endpoint for bot go get individual memes count
  • Add new API endpoint for obtaining thanks leaderboard

DB changes

  • Adds DiscordUserActivity, a generic table tracks actions users can apply to each other such as liking memes or thanking each other
  • Adds several methods to DiscordUser to query DiscorduserActivity
  • Adds start_date to Dojo object, which currently returns first earliest visible module as a proxy for course start

Course Grades page Changes

  • Adds assignment type helpfulness and memes
    • Both of these have an optional, but necessary max_credit property
      • sets individual max limits if course requires
      • Default is 1.00 max (100% of course)
    • helpfulness has a functionally required method property identifying the calculation method to use
      • Current valid values:
        • 1337log2 uses 1.337 ** log2(thanks) / 100 with a hard cap of max_credit if set
        • log50 uses log50 approaching the max_credit value
  • Adds course.yml course level property ec_limit
    • Serves as a dynamic global cap on the total earnable extra credit (across thanks, memes, and extra)
    • Extra credit values on the grades page will be dynamically reduced to ensure total cannot exceed this threshold
  • Adds dynamic progress indicators for helpfulness (total count) and memes (week ranges)
  • Meme and helpfulness grade time frames are calculated as dojo.start_date (described above) to start_date + 16 weeks

I believe this setup is sufficient to represent all currently live ASU EC scenarios based on course syllabus.

dojo_plugin/api/v1/discord.py Outdated Show resolved Hide resolved
@robwaz
Copy link
Member Author

robwaz commented Oct 3, 2024

Oh yeah.. and I also made DiscordUser IDs numbers.. because they are.

@robwaz
Copy link
Member Author

robwaz commented Oct 3, 2024

Couple things I realized this morning I need to address before merging.

  • DiscordAction Timestamp is "time of action", but we also want "time of target message" (ex: when meme was posted)
  • Similarly, the meme grade calculation should use "time of message", not "time of action"
  • Do we want thanks calculation to be "time of message" as well? Less important, but that'd be consistent
  • Update memes and thanks queries to be unique on message_id or (channel_id, message_id). I do think it's worth storing duplicate actions in the DB for accurate data, but de-duping for grades.

dojo_plugin/api/v1/discord.py Outdated Show resolved Hide resolved
@sjzhu
Copy link
Contributor

sjzhu commented Oct 3, 2024

I have no authority to approve this, but LGTM other than that one apparent typo for the thanks leaderboard calc!

Couple things I realized this morning I need to address before merging.

  • DiscordAction Timestamp is "time of action", but we also want "time of target message" (ex: when meme was posted)
  • Similarly, the meme grade calculation should use "time of message", not "time of action"
  • Do we want thanks calculation to be "time of message" as well? Less important, but that'd be consistent
  • Update memes and thanks queries to be unique on message_id or (channel_id, message_id). I do think it's worth storing duplicate actions in the DB for accurate data, but de-duping for grades.

Agree with the first 3 of these. Although I'm not sure what purpose the last point has? AFAICT, all discord message ids are unique, even between channels, so being unique on just message_id should be sufficient?

@robwaz
Copy link
Member Author

robwaz commented Oct 4, 2024

Agree with the first 3 of these. Although I'm not sure what purpose the last point has? AFAICT, all discord message ids are unique, even between channels, so being unique on just message_id should be sufficient?

From the Discord API docs

These IDs are guaranteed to be unique across all of Discord, except in some unique scenarios in which child objects share their parent's ID.

You're probably correct that just being distinct on message _id is sufficient. I was not aware the discord guaranteed uniqueness across all of discord (mostly).

@robwaz
Copy link
Member Author

robwaz commented Oct 4, 2024

@ConnorNelson any thoughts? I'd like merge this in late tonight.

dojo_plugin/models/__init__.py Show resolved Hide resolved
dojo_plugin/models/__init__.py Outdated Show resolved Hide resolved
dojo_plugin/models/__init__.py Outdated Show resolved Hide resolved
dojo_plugin/models/__init__.py Outdated Show resolved Hide resolved
dojo_plugin/models/__init__.py Outdated Show resolved Hide resolved
@@ -24,6 +25,17 @@ def get_letter_grade(dojo, grade):
return letter_grade
return "?"

def clamp_ec(limit):
global_limit = [limit]
Copy link
Member

Choose a reason for hiding this comment

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

Am I confused? Why is this a list?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is some insane python thing with the decorator instance holding the limiting value across functions. If it is not a list the value isn't captured in the decorator.

Copy link
Member

Choose a reason for hiding this comment

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

This is how scoping works inside python, what you're looking for is the nonlocal keyword.

I believe this is what you are trying to express:

def clamp_ec(limit):
    def decorator(func):
        def wrapper(*args, **kwargs):
            nonlocal limit
            result = func(*args, **kwargs)
            clamped_result = min(result, limit)
            limit -= clamped_result
            return clamped_result
        return wrapper
    return decorator

dojo_plugin/api/v1/discord.py Outdated Show resolved Hide resolved
dojo_plugin/api/v1/discord.py Outdated Show resolved Hide resolved
@robwaz
Copy link
Member Author

robwaz commented Oct 8, 2024

YOLO

@robwaz robwaz merged commit 929e31f into master Oct 8, 2024
1 check passed
@robwaz robwaz deleted the feat/thanks branch October 20, 2024 19:09
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

Successfully merging this pull request may close these issues.

3 participants