Skip to content

Amax reduction interval - #154

Merged
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval
Apr 18, 2023
Merged

Amax reduction interval#154
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval

Conversation

@erhoo82

Copy link
Copy Markdown
Collaborator

Set the DP-domain AMAX reduction interval by setting an integer value to NVTE_DP_AMAX_REDUCE_INTERVAL.
For example, when setting NVTE_DP_AMAX_REDUCE_INTERVAL=8, 7/8 instances do the reduction in TP-domain only and 1/8 does a reduction in both TP and DP domain.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should explicitly pass in self.tp_group_initialized since group=None refers to the world process group in torch.distributed.

@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Isn't it either TP group or AMAX reduction group provided by a user?

@ksivaman

Copy link
Copy Markdown
Member

The user provides the fp8_group which is the group across which they want amaxes to be reduced. Another concern here is that this adds some additional semantics w.r.t what the makeup of the provided group is. Since all this is undocumented and only exposed via envvars, maybe it's fine, but not ideal. Tim's suggestion is that we cannot blindly check for None since it's a valid arg for the user to pass in case they want global reduction.

Comment threadtransformer_engine/pytorch/fp8.py Outdated

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once the tests are green.

Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Thanks for the comment. I haven't tested this yet. Will reflect feedback.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
@erhoo82
erhoo82force-pushed the slym/amax_reduce_interval branch from f44b26d to dd4b8f6CompareApril 18, 2023 03:27
@ksivamanksivaman changed the title Draft: Amax reduction internvalAmax reduction intervalApr 18, 2023
@ksivaman

Copy link
Copy Markdown
Member

/te-ci

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ksivaman
ksivaman merged commit d3d7ed2 into NVIDIA:mainApr 18, 2023
ptrendx pushed a commit that referenced this pull request Apr 25, 2023
* amax reduction internval
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* Update transformer_engine/pytorch/fp8.py
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* check TP group initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
---------
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
@erhoo82
erhoo82 deleted the slym/amax_reduce_interval branch January 6, 2024 09:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@erhoo82@ksivaman@timmoon10
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Amax reduction interval by erhoo82 · Pull Request #154 · NVIDIA/TransformerEngine · GitHub
Skip to content

Amax reduction interval - #154

Merged
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval
Apr 18, 2023
Merged

Amax reduction interval#154
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval

Conversation

@erhoo82

Copy link
Copy Markdown
Collaborator

Set the DP-domain AMAX reduction interval by setting an integer value to NVTE_DP_AMAX_REDUCE_INTERVAL.
For example, when setting NVTE_DP_AMAX_REDUCE_INTERVAL=8, 7/8 instances do the reduction in TP-domain only and 1/8 does a reduction in both TP and DP domain.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should explicitly pass in self.tp_group_initialized since group=None refers to the world process group in torch.distributed.

@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Isn't it either TP group or AMAX reduction group provided by a user?

@ksivaman

Copy link
Copy Markdown
Member

The user provides the fp8_group which is the group across which they want amaxes to be reduced. Another concern here is that this adds some additional semantics w.r.t what the makeup of the provided group is. Since all this is undocumented and only exposed via envvars, maybe it's fine, but not ideal. Tim's suggestion is that we cannot blindly check for None since it's a valid arg for the user to pass in case they want global reduction.

Comment threadtransformer_engine/pytorch/fp8.py Outdated

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once the tests are green.

Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Thanks for the comment. I haven't tested this yet. Will reflect feedback.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
@erhoo82
erhoo82force-pushed the slym/amax_reduce_interval branch from f44b26d to dd4b8f6CompareApril 18, 2023 03:27
@ksivamanksivaman changed the title Draft: Amax reduction internvalAmax reduction intervalApr 18, 2023
@ksivaman

Copy link
Copy Markdown
Member

/te-ci

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ksivaman
ksivaman merged commit d3d7ed2 into NVIDIA:mainApr 18, 2023
ptrendx pushed a commit that referenced this pull request Apr 25, 2023
* amax reduction internval
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* Update transformer_engine/pytorch/fp8.py
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* check TP group initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
---------
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
@erhoo82
erhoo82 deleted the slym/amax_reduce_interval branch January 6, 2024 09:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@erhoo82@ksivaman@timmoon10
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Amax reduction interval by erhoo82 · Pull Request #154 · NVIDIA/TransformerEngine · GitHub
Skip to content

Amax reduction interval - #154

Merged
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval
Apr 18, 2023
Merged

Amax reduction interval#154
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval

Conversation

@erhoo82

Copy link
Copy Markdown
Collaborator

Set the DP-domain AMAX reduction interval by setting an integer value to NVTE_DP_AMAX_REDUCE_INTERVAL.
For example, when setting NVTE_DP_AMAX_REDUCE_INTERVAL=8, 7/8 instances do the reduction in TP-domain only and 1/8 does a reduction in both TP and DP domain.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should explicitly pass in self.tp_group_initialized since group=None refers to the world process group in torch.distributed.

@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Isn't it either TP group or AMAX reduction group provided by a user?

@ksivaman

Copy link
Copy Markdown
Member

The user provides the fp8_group which is the group across which they want amaxes to be reduced. Another concern here is that this adds some additional semantics w.r.t what the makeup of the provided group is. Since all this is undocumented and only exposed via envvars, maybe it's fine, but not ideal. Tim's suggestion is that we cannot blindly check for None since it's a valid arg for the user to pass in case they want global reduction.

Comment threadtransformer_engine/pytorch/fp8.py Outdated

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once the tests are green.

Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Thanks for the comment. I haven't tested this yet. Will reflect feedback.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
@erhoo82
erhoo82force-pushed the slym/amax_reduce_interval branch from f44b26d to dd4b8f6CompareApril 18, 2023 03:27
@ksivamanksivaman changed the title Draft: Amax reduction internvalAmax reduction intervalApr 18, 2023
@ksivaman

Copy link
Copy Markdown
Member

/te-ci

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ksivaman
ksivaman merged commit d3d7ed2 into NVIDIA:mainApr 18, 2023
ptrendx pushed a commit that referenced this pull request Apr 25, 2023
* amax reduction internval
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* Update transformer_engine/pytorch/fp8.py
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* check TP group initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
---------
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
@erhoo82
erhoo82 deleted the slym/amax_reduce_interval branch January 6, 2024 09:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@erhoo82@ksivaman@timmoon10
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Amax reduction interval by erhoo82 · Pull Request #154 · NVIDIA/TransformerEngine · GitHub
Skip to content

Amax reduction interval - #154

Merged
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval
Apr 18, 2023
Merged

Amax reduction interval#154
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval

Conversation

@erhoo82

Copy link
Copy Markdown
Collaborator

Set the DP-domain AMAX reduction interval by setting an integer value to NVTE_DP_AMAX_REDUCE_INTERVAL.
For example, when setting NVTE_DP_AMAX_REDUCE_INTERVAL=8, 7/8 instances do the reduction in TP-domain only and 1/8 does a reduction in both TP and DP domain.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should explicitly pass in self.tp_group_initialized since group=None refers to the world process group in torch.distributed.

@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Isn't it either TP group or AMAX reduction group provided by a user?

@ksivaman

Copy link
Copy Markdown
Member

The user provides the fp8_group which is the group across which they want amaxes to be reduced. Another concern here is that this adds some additional semantics w.r.t what the makeup of the provided group is. Since all this is undocumented and only exposed via envvars, maybe it's fine, but not ideal. Tim's suggestion is that we cannot blindly check for None since it's a valid arg for the user to pass in case they want global reduction.

Comment threadtransformer_engine/pytorch/fp8.py Outdated

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once the tests are green.

Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Thanks for the comment. I haven't tested this yet. Will reflect feedback.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
@erhoo82
erhoo82force-pushed the slym/amax_reduce_interval branch from f44b26d to dd4b8f6CompareApril 18, 2023 03:27
@ksivamanksivaman changed the title Draft: Amax reduction internvalAmax reduction intervalApr 18, 2023
@ksivaman

Copy link
Copy Markdown
Member

/te-ci

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ksivaman
ksivaman merged commit d3d7ed2 into NVIDIA:mainApr 18, 2023
ptrendx pushed a commit that referenced this pull request Apr 25, 2023
* amax reduction internval
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* Update transformer_engine/pytorch/fp8.py
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* check TP group initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
---------
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
@erhoo82
erhoo82 deleted the slym/amax_reduce_interval branch January 6, 2024 09:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@erhoo82@ksivaman@timmoon10
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Amax reduction interval by erhoo82 · Pull Request #154 · NVIDIA/TransformerEngine · GitHub
Skip to content

Amax reduction interval - #154

Merged
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval
Apr 18, 2023
Merged

Amax reduction interval#154
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval

Conversation

@erhoo82

Copy link
Copy Markdown
Collaborator

Set the DP-domain AMAX reduction interval by setting an integer value to NVTE_DP_AMAX_REDUCE_INTERVAL.
For example, when setting NVTE_DP_AMAX_REDUCE_INTERVAL=8, 7/8 instances do the reduction in TP-domain only and 1/8 does a reduction in both TP and DP domain.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should explicitly pass in self.tp_group_initialized since group=None refers to the world process group in torch.distributed.

@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Isn't it either TP group or AMAX reduction group provided by a user?

@ksivaman

Copy link
Copy Markdown
Member

The user provides the fp8_group which is the group across which they want amaxes to be reduced. Another concern here is that this adds some additional semantics w.r.t what the makeup of the provided group is. Since all this is undocumented and only exposed via envvars, maybe it's fine, but not ideal. Tim's suggestion is that we cannot blindly check for None since it's a valid arg for the user to pass in case they want global reduction.

Comment threadtransformer_engine/pytorch/fp8.py Outdated

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once the tests are green.

Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Thanks for the comment. I haven't tested this yet. Will reflect feedback.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
@erhoo82
erhoo82force-pushed the slym/amax_reduce_interval branch from f44b26d to dd4b8f6CompareApril 18, 2023 03:27
@ksivamanksivaman changed the title Draft: Amax reduction internvalAmax reduction intervalApr 18, 2023
@ksivaman

Copy link
Copy Markdown
Member

/te-ci

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ksivaman
ksivaman merged commit d3d7ed2 into NVIDIA:mainApr 18, 2023
ptrendx pushed a commit that referenced this pull request Apr 25, 2023
* amax reduction internval
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* Update transformer_engine/pytorch/fp8.py
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* check TP group initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
---------
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
@erhoo82
erhoo82 deleted the slym/amax_reduce_interval branch January 6, 2024 09:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@erhoo82@ksivaman@timmoon10
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Amax reduction interval by erhoo82 · Pull Request #154 · NVIDIA/TransformerEngine · GitHub
Skip to content

Amax reduction interval - #154

Merged
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval
Apr 18, 2023
Merged

Amax reduction interval#154
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval

Conversation

@erhoo82

Copy link
Copy Markdown
Collaborator

Set the DP-domain AMAX reduction interval by setting an integer value to NVTE_DP_AMAX_REDUCE_INTERVAL.
For example, when setting NVTE_DP_AMAX_REDUCE_INTERVAL=8, 7/8 instances do the reduction in TP-domain only and 1/8 does a reduction in both TP and DP domain.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should explicitly pass in self.tp_group_initialized since group=None refers to the world process group in torch.distributed.

@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Isn't it either TP group or AMAX reduction group provided by a user?

@ksivaman

Copy link
Copy Markdown
Member

The user provides the fp8_group which is the group across which they want amaxes to be reduced. Another concern here is that this adds some additional semantics w.r.t what the makeup of the provided group is. Since all this is undocumented and only exposed via envvars, maybe it's fine, but not ideal. Tim's suggestion is that we cannot blindly check for None since it's a valid arg for the user to pass in case they want global reduction.

Comment threadtransformer_engine/pytorch/fp8.py Outdated

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once the tests are green.

Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Thanks for the comment. I haven't tested this yet. Will reflect feedback.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
@erhoo82
erhoo82force-pushed the slym/amax_reduce_interval branch from f44b26d to dd4b8f6CompareApril 18, 2023 03:27
@ksivamanksivaman changed the title Draft: Amax reduction internvalAmax reduction intervalApr 18, 2023
@ksivaman

Copy link
Copy Markdown
Member

/te-ci

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ksivaman
ksivaman merged commit d3d7ed2 into NVIDIA:mainApr 18, 2023
ptrendx pushed a commit that referenced this pull request Apr 25, 2023
* amax reduction internval
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* Update transformer_engine/pytorch/fp8.py
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* check TP group initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
---------
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
@erhoo82
erhoo82 deleted the slym/amax_reduce_interval branch January 6, 2024 09:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@erhoo82@ksivaman@timmoon10
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Amax reduction interval by erhoo82 · Pull Request #154 · NVIDIA/TransformerEngine · GitHub
Skip to content

Amax reduction interval - #154

Merged
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval
Apr 18, 2023
Merged

Amax reduction interval#154
ksivaman merged 4 commits into
NVIDIA:mainfrom
erhoo82:slym/amax_reduce_interval

Conversation

@erhoo82

Copy link
Copy Markdown
Collaborator

Set the DP-domain AMAX reduction interval by setting an integer value to NVTE_DP_AMAX_REDUCE_INTERVAL.
For example, when setting NVTE_DP_AMAX_REDUCE_INTERVAL=8, 7/8 instances do the reduction in TP-domain only and 1/8 does a reduction in both TP and DP domain.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should explicitly pass in self.tp_group_initialized since group=None refers to the world process group in torch.distributed.

@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Isn't it either TP group or AMAX reduction group provided by a user?

@ksivaman

Copy link
Copy Markdown
Member

The user provides the fp8_group which is the group across which they want amaxes to be reduced. Another concern here is that this adds some additional semantics w.r.t what the makeup of the provided group is. Since all this is undocumented and only exposed via envvars, maybe it's fine, but not ideal. Tim's suggestion is that we cannot blindly check for None since it's a valid arg for the user to pass in case they want global reduction.

Comment threadtransformer_engine/pytorch/fp8.py Outdated

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once the tests are green.

Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/fp8.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
Comment threadtransformer_engine/pytorch/module.py Outdated
@erhoo82

Copy link
Copy Markdown
CollaboratorAuthor

Thanks for the comment. I haven't tested this yet. Will reflect feedback.

Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
@erhoo82
erhoo82force-pushed the slym/amax_reduce_interval branch from f44b26d to dd4b8f6CompareApril 18, 2023 03:27
@ksivamanksivaman changed the title Draft: Amax reduction internvalAmax reduction intervalApr 18, 2023
@ksivaman

Copy link
Copy Markdown
Member

/te-ci

@timmoon10timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ksivaman
ksivaman merged commit d3d7ed2 into NVIDIA:mainApr 18, 2023
ptrendx pushed a commit that referenced this pull request Apr 25, 2023
* amax reduction internval
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Skip TP-domain only AMAX reduction when TP-group is not initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* Update transformer_engine/pytorch/fp8.py
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Sangkug Lym <slym@nvidia.com>
* check TP group initialized
Signed-off-by: Sangkug Lym <slym@nvidia.com>
fix
Signed-off-by: Sangkug Lym <slym@nvidia.com>
---------
Signed-off-by: Sangkug Lym <slym@nvidia.com>
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
@erhoo82
erhoo82 deleted the slym/amax_reduce_interval branch January 6, 2024 09:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@erhoo82@ksivaman@timmoon10