Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

remove three other roundings in cpu calculation#2807

Open
nwangtw wants to merge 1 commit into
apache:masterfrom
nwangtw:nwang/remove_more_rounding_in_pp_builder
Open

remove three other roundings in cpu calculation#2807
nwangtw wants to merge 1 commit into
apache:masterfrom
nwangtw:nwang/remove_more_rounding_in_pp_builder

Conversation

@nwangtw

@nwangtw nwangtw commented Mar 23, 2018

Copy link
Copy Markdown
Contributor

No description provided.

@nwangtw nwangtw requested a review from avflor March 23, 2018 21:42
@nwangtw

nwangtw commented Mar 23, 2018

Copy link
Copy Markdown
Contributor Author

unit tests to update

@nlu90

nlu90 commented May 18, 2018

Copy link
Copy Markdown
Member

What's the background of removing this Math.rounding()?

@nwangtw

nwangtw commented May 18, 2018

Copy link
Copy Markdown
Contributor Author

It is more about "why there was a Math.rounding()".

The rounding logic doesn't seem necessary if we assume cpu # can be less than fractional. Also the logic have bugs that in some cases it can be floored to 0.

However when I was working on this PR, I found out that some parking algorithms rely on the rounding so this is not a quick fix. This PR needs to be revisited when I get more time.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants