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

Pull in tag sets from various PRs #165

Merged
merged 3 commits into from
Oct 17, 2018
Merged

Pull in tag sets from various PRs #165

merged 3 commits into from
Oct 17, 2018

Conversation

robieta
Copy link
Contributor

@robieta robieta commented Oct 17, 2018

I noticed that several PRs have merge conflicts due to the recent change in compliance utils structure. To make everyone's lives easier, I have pulled all of the changes into a util specific PR and will build 0.0.4 of the mlperf_compliance pip package once this PR is merged. That way you don't have to deal with resolving the merge confilcts; just add a line to requirements.txt

Notable changes:
GNMT: @szmigacz
#157

  • I changed some tag names in _gnmt_tags.py to match the tag format.
  • I did not include the path manipulation for setting ROOT_DIR_GNMT. Please override that value in a main function.

SSD: @christ1ne
#159

  • Just pulled in the expanded tag set. (Thanks for that!)

Transformer: @xyhuang

General:

  • Changed some relative paths to absolute paths.
  • Added a simple test file to check for tag collisions and perform a basic tag name check.

I couldn't add @szmigacz as a reviewer, but let me know if you're fine with the GNMT changes.

@szmigacz
Copy link
Contributor

I'm ok with the GNMT changes (tag names and ROOT_DIR_GNMT).
I noticed one issue: mlperf_log.py doesn't define "gnmt_print" function, this function can be copied from #157.

@christ1ne
Copy link
Contributor

looks good for ssd & minigo

@robieta
Copy link
Contributor Author

robieta commented Oct 17, 2018

I'm ok with the GNMT changes (tag names and ROOT_DIR_GNMT).
I noticed one issue: mlperf_log.py doesn't define "gnmt_print" function, this function can be copied from #157.

Fixed. Thanks!

christ1ne
christ1ne previously approved these changes Oct 17, 2018
Copy link
Contributor

@xyhuang xyhuang left a comment

Choose a reason for hiding this comment

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

Hi Taylor, I need a small change (see comment), otherwise LGTM. thanks.

compliance/mlperf_compliance/mlperf_log.py Outdated Show resolved Hide resolved
@robieta
Copy link
Contributor Author

robieta commented Oct 17, 2018

Alright! Changes are in PyPi. Thanks everyone!

@robieta robieta merged commit 055a270 into mlcommons:master Oct 17, 2018
dagarcia-nvidia pushed a commit to dagarcia-nvidia/training that referenced this pull request Feb 27, 2019
* fix some minor issues in dockerfile


RUN echo 'debconf debconf/frontend select Noninteractive' | debconf-set-selections
fixes
debconf: unable to initialize frontend: Dialog
debconf: (TERM is not set, so the dialog frontend is not usable.)
debconf: falling back to frontend: Readline
debconf: unable to initialize frontend: Readline
debconf: (This frontend requires a controlling tty.)
debconf: falling back to frontend: Teletype
dpkg-preconfigure: unable to re-open stdin: 

RUN apt-get update -y && apt-get install -y apt-utils
fixes
debconf: delaying package configuration, since apt-utils is not installed

* remove redundant command

* add -y to conda install and pip install

* remove -y from pip install
ekrimer pushed a commit to ekrimer/training that referenced this pull request Oct 10, 2019
* Pull in tag sets from various PRs

* add gnmt print function

* fix stack offset
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.

None yet

4 participants