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.

Sign up for GitHub

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

Jump to bottom

[datasets] Small benchmark URI fix and tweak #555

Merged
merged 2 commits into from
Feb 7, 2022
Merged

[datasets] Small benchmark URI fix and tweak #555

merged 2 commits into from
Feb 7, 2022

Conversation

ChrisCummins
Copy link
Contributor

@ChrisCummins ChrisCummins commented Jan 27, 2022 •
edited
Loading

This fixes a typo and test regression introduced in #524.

@ChrisCummins
[examples] Fix typo in example method override.
01cb0be
@ChrisCummins
[datasets] Add startswith() and endswith() to Benchmark class.
78783a0 
This provides methods to forward startswith() and endswith() calls to
the underlying string representation.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 27, 2022
@ChrisCummins ChrisCummins added this to the v0.2.3 milestone Jan 27, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 27, 2022 •
edited
Loading

Codecov Report

Merging #555 (78783a0) into development (51e3c0a) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #555      +/-   ##
===============================================
+ Coverage        87.88%   87.96%   +0.08%     
===============================================
  Files              113      113              
  Lines             6486     6490       +4     
===============================================
+ Hits              5700     5709       +9     
+ Misses             786      781       -5
Impacted Files Coverage Δ
compiler_gym/datasets/uri.py 93.18% <100.00%> (+0.68%) ⬆️
...loop_tool/service/loop_tool_compilation_session.py 88.59% <0.00%> (-0.68%) ⬇️
compiler_gym/envs/compiler_env.py 91.60% <0.00%> (-0.50%) ⬇️
compiler_gym/service/connection.py 79.93% <0.00%> (+2.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51e3c0a...78783a0. Read the comment docs.

@ChrisCummins ChrisCummins requested a review from hughleat February 7, 2022 17:35
hughleat
hughleat approved these changes Feb 7, 2022
View reviewed changes
Copy link
Contributor

@hughleat hughleat left a comment

Choose a reason for hiding this comment

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

LGTM

If this string processing is frequent and creating then unparsing is expensive, consider caching __repr__?

@ChrisCummins
Copy link
Contributor Author

ChrisCummins commented Feb 7, 2022 •
edited
Loading

Re. Caching __repr__, good idea! I'll do this

@ChrisCummins ChrisCummins merged commit 67ff153 into facebookresearch:development Feb 7, 2022
@ChrisCummins ChrisCummins deleted the fix/benchmark-uri branch February 7, 2022 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hughleat hughleat hughleat approved these changes

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Milestone
v0.2.3
Development

Successfully merging this pull request may close these issues.
4 participants
@ChrisCummins @codecov-commenter @hughleat @facebook-github-bot