Recommend Schedule Endpoint#444
Conversation
AaDalal
left a comment
AaDalal
left a comment
There was a problem hiding this comment.
Overall this is a really interesting feature! I left a few comments, but most of them are questions.
A small note: you also need to lint the PR before we merge.
| response_codes={ | ||
| reverse_func("recommend-schedule"): { | ||
| "POST": { | ||
| 200: "[DESCRIBE_RESPONSE_SCHEMA]Response returned successfully.", |
There was a problem hiding this comment.
Do you know what tag this should fall under in the auto generated docs?
There was a problem hiding this comment.
If you don't specify one, it will show up under [PCP] Course Recommendations which is probably correct.
| reverse_func("recommend-schedule"): { | ||
| "POST": { | ||
| 200: "[DESCRIBE_RESPONSE_SCHEMA]Response returned successfully.", | ||
| 201: "[UNDOCUMENTED]", |
There was a problem hiding this comment.
What does HTTP 201 "Created" mean here?
| "The times that sections in the recommended schedule can have " | ||
| "meetings within. The start and end time of the filter should be " | ||
| "dash-separated. Times should be specified as decimal numbers of " | ||
| "the form `h+mm/100` where h is the hour `[0..23]` and mm is the " |
There was a problem hiding this comment.
Also, are the hour and minute separated by a + or by a .? I think you should clarify in this description.
| ), | ||
| }, | ||
| "time": { | ||
| "type": "string", |
There was a problem hiding this comment.
Should this be a string or a decimal number?
| "attributes": { | ||
| "type": "array", | ||
| "description": ( | ||
| "An array of attributes that must be fulfilled by the recommended " |
There was a problem hiding this comment.
What does it mean to fulfill an attribute?
| model.AddImplication(sections[section.full_code], courses[course.full_code]) | ||
| model.AddImplication(courses[course.full_code].Not(), sections[section.full_code].Not()) | ||
|
|
||
| if section.full_code in locked_sections: |
There was a problem hiding this comment.
Why is it necessary to handle locked_sections both above (ie in constructing the query set) and here?
| section_score += section.instructor_quality or 0.0 | ||
| if min_difficulty: | ||
| section_score -= course.difficulty or 4.0 | ||
| section_score = int(section_score * int(100 * section.credits)) |
There was a problem hiding this comment.
why weight by section credit?
| "The maximum number of courses for the recommended schedule. " | ||
| ), | ||
| }, | ||
| "locked_courses": { |
There was a problem hiding this comment.
Can people specify if they only want to consider a specific set of courses?
| for req in attribute_requirements: | ||
| model.Add(sum(schedule_attributes[req["code"]]) == req["num"]) | ||
|
|
||
| if max_quality or min_difficulty: |
There was a problem hiding this comment.
Does it make sense to have a generic scoring mechanism, so we can provide the most optimal schedules?
|
|
||
| # Solve the model | ||
| solver = cp_model.CpSolver() | ||
| solver.parameters.max_time_in_seconds = 10.0 |
There was a problem hiding this comment.
Does it make sense to make this an async celery task? I'm not sure what the performance implications of the having this solver run for 10+ seconds is.
3 participants
This pull request creates a route (
/api/plan/recommendations/schedule/) that generates a schedule given a number of constraints.