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

Switch EmbeddingOffloadScaleupProposer to use Luus Jaakola search #1691

Closed
wants to merge 2 commits into from

Conversation

damianr99
Copy link

Summary:
Replace binary-search with Luus Jaakola. This works better at
navigating the non-smooth planner cost surface when exploring cost vs
cache memory.

Reviewed By: henrylhtsang

Differential Revision: D53296719

Damian Reeves added 2 commits February 7, 2024 13:48
…orch#1690)

Summary:

EmbeddingOffloadScaleupProposer attempts to find an approximately
optimal trade-off between using extra cache memory to reduce prefetch
delay and the additional plan cost resulting from a more challenging
bin-packing of larger shards.

Currently a binary search is used under the assumption that the
cost would consistently decrease in the evaluated region. However,
investigation revealed that the cost region can be non-smooth and
multi-modal, leading to poor results with the binary search.

This diff implements the Luus Jaakola search procedure which is much
more robust and capable of navigating 'rough terrain'. In a subsequent
diff EmbeddingOffloadScaleupProposer will be updated to utilize this
approach instead of the binary search.

See https://en.wikipedia.org/wiki/Luus-Jaakola for detail.

Reviewed By: henrylhtsang

Differential Revision: D52686075
Summary:
Replace binary-search with Luus Jaakola. This works better at
navigating the non-smooth planner cost surface when exploring cost vs
cache memory.

Reviewed By: henrylhtsang

Differential Revision: D53296719
@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 Feb 7, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D53296719

damianr99 pushed a commit to damianr99/torchrec that referenced this pull request Feb 7, 2024
…torch#1691)

Summary:

Replace binary-search with Luus Jaakola. This works better at
navigating the non-smooth planner cost surface when exploring cost vs
cache memory.

Reviewed By: henrylhtsang

Differential Revision: D53296719
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants