Don't allow PCs to compute affinity scores if there are over 2k submissions #2041
Conversation
| pretty_role = role_name.replace('_', ' ') | ||
|
|
||
| if compute_affinity_scores and 'upload_affinity_scores' in note.content: |
There was a problem hiding this comment.
I would leave this if before checking the submission count that is needed later.
|
Could you please fix this issue in this PR too:#2016 it will require changing the preprocess function too. |
|
Yes, I will also fix #2015 in this PR |
enrubio
changed the title
| domain = client_v2.get_group(venue_id) | ||
| submission_venue_id = domain.content['submission_venue_id']['value'] | ||
| senior_area_chairs_name = domain.content.get('senior_area_chairs_name', {}).get('value') | ||
| compute_affinity_scores = note.content.get('compute_affinity_scores') == 'Yes' |
There was a problem hiding this comment.
can you check the value No instead? we are going to add more options that represent the affinity model but "No" will stay: https://github.com/openreview/openreview-py/pull/1843/files
| if compute_affinity_scores and 'upload_affinity_scores' in note.content: |
There was a problem hiding this comment.
I guess you are missing not compute_affinity_scores is it is reading the yes value.
|
|
||
| matching_group = note.content['matching_group'] | ||
| role_name = matching_group.split('/')[-1] |
There was a problem hiding this comment.
instead of parsing the matching_group can you check if matching.group.ends_with(senior_area_chairs_name)?
| role_name = matching_group.split('/')[-1] | ||
| pretty_role = role_name.replace('_', ' ') | ||
| _, num_submissions = client_v2.get_notes(content={ 'venueid':submission_venue_id }, limit=1, with_count=True) |
There was a problem hiding this comment.
you don't need to get the notes if the role is SAC, so you can check:
if matching.group.ends_with(senior_area_chairs_name):
return
...get_notes()
| matching_group = note.content['matching_group'] | ||
| senior_area_chairs_name = domain.get_content_value('senior_area_chairs_name') | ||
| if senior_area_chairs_name and matching_group.endswith(senior_area_chairs_name): return |
There was a problem hiding this comment.
There is no Python convention about this but our code put the return statement in a new line. We would like to keep consistency in the code style.
|
The changes look good! |
| matching_group = note.content['matching_group'] | ||
| senior_area_chairs_name = domain.get_content_value('senior_area_chairs_name') | ||
| if matching_group.endswith(senior_area_chairs_name): |
There was a problem hiding this comment.
I'm not sure why you deleted the check that senior_area_chairs_name is not None.
There was a problem hiding this comment.
Oops, I thought the senior_area_chairs_name was always a part of the domain and thought it was redundant. Fixed now.
There was a problem hiding this comment.
senior_area_chairs_name is only present if the venue has SAC.
Edit:
Decided to fix the other paper matching issues in separate PRs.