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

Peer Review Rules #290

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Peer Review Rules #290

wants to merge 5 commits into from

Conversation

mrmhodak
Copy link
Contributor

@mrmhodak mrmhodak commented Feb 6, 2024

No description provided.

@mrmhodak mrmhodak requested a review from a team as a code owner February 6, 2024 07:30
Copy link

github-actions bot commented Feb 6, 2024

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@arjunsuresh
Copy link
Contributor

arjunsuresh commented Feb 6, 2024

Thank you @mrmhodak for formulating this. I would like to add the below point too.

Any submitter can request for access to a submission system to reproduce the submission. If the submitter grants the access and in case the reviewer is satisfied with his reproducibility work, this process can be used to waive (need not be binding) the system from the official audit.

@mrmhodak
Copy link
Contributor Author

mrmhodak commented Feb 7, 2024

Arjun, that is an interesting proposition. I will bring it up for discussion in the WG leadership first.

inference_rules.adoc Show resolved Hide resolved

* All submitters are compiled into an ordered list. Alphabetical, or any other way order is fine

* A list randomizer is used to generate a re-ordered list live during a review meeting. List Randomizer from random.org can be used.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we simplify the wording here? I suspect it is easier if we just said "submitters will be randomly assigned another to review"... I think as long as it's random, we don't need to get into implementation details in the rules.


* During the review, reviewers are asked to pay special attention to: (1) results validity, (2) methodology, (3) instructions for reproducibility, and (4) content of json files in systems directory.

* Any issues discovered should be filed as github issues and resolved as usual. The issues should be filed promptly before the issue filing deadline.
Copy link
Contributor

Choose a reason for hiding this comment

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

While I 100% agree with the importantance of "The issues should be filed promptly before the issue filing deadline." I'd like to limit the number of suggestions we write as rules. What to you think of combining all the suggestions / encouragements into a single statement like,

"Reviewers will be given these directions:

  • You are encouraged to pay special attention to: (1) results validity, (2) methodology, (3) instructions for reproducibility, and (4) content of json files in systems directory.
  • You are encouraged to file your review early
  • You are encouraged to review other submitters beyond your assignments."


* Peer Reviews are assigned based on the new order with the last submitter in the new order assigned to review the first one. For example, with companies Company1, Company2, and Company3 participating, the list can be reordered to Company2, Company1, Company3. The Assigned Reviews will be: Company2 will review Company1, Company1 will review Company3, Company3 will review Company2.

* Chair will open a github issue against each company that has review assignment. Issues will be closed once reviewers indicate that they had finished their tasks.
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with the github issue process, but again don't want to prescribe the process into the rules. Perhaps this could go into a 1 pager document off the rules for future reference.

I think it is okay to leave these exact details to the discretion of the chair and working group to adjust as necessary and is practical.

@bitfort
Copy link
Contributor

bitfort commented Feb 14, 2024

Any submitter can ask for access to a submission system to reproduce the submission.

Very interesting idea. One clarifying question here: "can ask" implies that the submitter asked can say "no.". I think clarifying right of refusal here is helpful in evaluating the language. Is it true that submitters can say "no" when asked?

@arjunsuresh
Copy link
Contributor

Any submitter can ask for access to a submission system to reproduce the submission.

Very interesting idea. One clarifying question here: "can ask" implies that the submitter asked can say "no.". I think clarifying right of refusal here is helpful in evaluating the language. Is it true that submitters can say "no" when asked?

Thank you @bitfort Yes, the submitter can say no. I just edited the comment to make this clear.

@mrmhodak
Copy link
Contributor Author

I have simplified the text per @bitfort.

Also, added @arjunsuresh suggestion on granting access and avoiding audit


* Submitters are encouraged to review other submissions beyond their assigned review.

During the Peer Review Process, a reviewer can ask for access to a submission system to reproduce the submission. If the request is granted and the reviewer is satisfied with the results, this can be used to waive the reviewed system from the audit, subject to the approval of the review group.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this language is good, "If the request is granted and the reviewer is satisfied with the results, this can be used to waive the reviewed system from the audit"

Because a supplier could use an OEM to self review and then exempt themselves from the audit. I'd support language where the random audit is random.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea of saving effort, but it seems too open ended and not specific. I'd favor simpler and easier rules.

Choose a reason for hiding this comment

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

What if its waived from the selection audit but not the random audit for this round?

Copy link
Contributor

Choose a reason for hiding this comment

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

@bitfort "subject to the approval of the review group" condition is still there. If the random audit is random, most of the time it is a waste of everyone's time like happening now.

@peladodigital The waiver is meant for random audit. Because the selection audit is done by the review committee and automatically the committee will be eliminating uninteresting and already reproduced submissions.

@mrmhodak
Copy link
Contributor Author

@bitfort :
I would like to merge this before Review Meetings start.
Excluding the last clause about audit, does the rest look good to you?

Removing waiving audit requirements
@mrmhodak
Copy link
Contributor Author

Pushed an update to remove the waiving audit clause. Too close to submission deadline to get an agreement.

To make it easy to find, the final version of the waiving audit clause was:

During the Peer Review Process, a reviewer can ask for access to a submission system to reproduce the submission. If the request is granted and the reviewer is satisfied with the results, this can be used to waive the reviewed system from the audit, subject to the approval of the review group

.

Copy link
Contributor

@bitfort bitfort left a comment

Choose a reason for hiding this comment

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

Thanks for the iteration here. I think this merits more discussion long term - but this looks workable as written for this round.

Appreciate your effort here.

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.

4 participants