Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix] - #4007

Merged
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning
Aug 5, 2019
Merged

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix]#4007
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning

Conversation

@rayankrish

Copy link
Copy Markdown

Issue #3965 reported that a warning, "LightGBM] [Warning] Unknown parameter metric=" is produced when the default metric is used. This warning came after this commit which aimed to provide a consistent user experience from an ML.NET implementation of LightGbm with standalone LightGbm. If a user were to set EvaluationMetric = EvaluateMetricType.Default, they might expect that this would set the EvaluationMetric to "" and assigned the metric based on the objective as shown in the LightGbm docs. When the correction was made, this warning began to appear when the metric parameter was set to "". which was also being produced in LightGbm alone. The only way to prevent this error would be to not assign a parameter to the metric at all.

This warning has not appeared in previous versions of ML.NET and can be prevent by assigning the correct metric based on the objective as was previously done.

To prevent this warning, the changes from this commit were reverted.

@najeeb-kazmi

Copy link
Copy Markdown
Member

If the only reason we are reverting the defaults is this warning that is not affecting training, we should weigh the two options:

  1. Keep the defaults at the expense of having this warning, which might cause confusion for the user.

  2. Revert defaults and avoid the warning at the expense of having inconsistent user experience compared to standalone LightGbm as described in LightGbm default evaluation metrics in ML.NET do not conform to standalone LightGbm #3822 and Change default EvaluationMetric for LightGbm trainers to conform to d… #3859, although this will also cause confusion for the user.

I have a preference for (1) from the point of view of a LightGbm user coming to ML.NET and finding that the defaults are different from what they are used to. That this warning is being printed when the correct LightGbm default i.e. "" is being passed to LightGbm seems to be an issue that should be fixed on the LightGbm side.

@rayankrish
rayankrish merged commit 820d359 into dotnet:masterAug 5, 2019
@ghostghost locked as resolved and limited conversation to collaborators Mar 21, 2022
Sign up for freeto 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.

3 participants

@rayankrish@najeeb-kazmi@sayanshaw24
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n 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;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix] - #4007

Merged
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning
Aug 5, 2019
Merged

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix]#4007
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning

Conversation

@rayankrish

Copy link
Copy Markdown

Issue #3965 reported that a warning, "LightGBM] [Warning] Unknown parameter metric=" is produced when the default metric is used. This warning came after this commit which aimed to provide a consistent user experience from an ML.NET implementation of LightGbm with standalone LightGbm. If a user were to set EvaluationMetric = EvaluateMetricType.Default, they might expect that this would set the EvaluationMetric to "" and assigned the metric based on the objective as shown in the LightGbm docs. When the correction was made, this warning began to appear when the metric parameter was set to "". which was also being produced in LightGbm alone. The only way to prevent this error would be to not assign a parameter to the metric at all.

This warning has not appeared in previous versions of ML.NET and can be prevent by assigning the correct metric based on the objective as was previously done.

To prevent this warning, the changes from this commit were reverted.

@najeeb-kazmi

Copy link
Copy Markdown
Member

If the only reason we are reverting the defaults is this warning that is not affecting training, we should weigh the two options:

  1. Keep the defaults at the expense of having this warning, which might cause confusion for the user.

  2. Revert defaults and avoid the warning at the expense of having inconsistent user experience compared to standalone LightGbm as described in LightGbm default evaluation metrics in ML.NET do not conform to standalone LightGbm #3822 and Change default EvaluationMetric for LightGbm trainers to conform to d… #3859, although this will also cause confusion for the user.

I have a preference for (1) from the point of view of a LightGbm user coming to ML.NET and finding that the defaults are different from what they are used to. That this warning is being printed when the correct LightGbm default i.e. "" is being passed to LightGbm seems to be an issue that should be fixed on the LightGbm side.

@rayankrish
rayankrish merged commit 820d359 into dotnet:masterAug 5, 2019
@ghostghost locked as resolved and limited conversation to collaborators Mar 21, 2022
Sign up for freeto 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.

3 participants

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

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix] - #4007

Merged
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning
Aug 5, 2019
Merged

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix]#4007
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning

Conversation

@rayankrish

Copy link
Copy Markdown

Issue #3965 reported that a warning, "LightGBM] [Warning] Unknown parameter metric=" is produced when the default metric is used. This warning came after this commit which aimed to provide a consistent user experience from an ML.NET implementation of LightGbm with standalone LightGbm. If a user were to set EvaluationMetric = EvaluateMetricType.Default, they might expect that this would set the EvaluationMetric to "" and assigned the metric based on the objective as shown in the LightGbm docs. When the correction was made, this warning began to appear when the metric parameter was set to "". which was also being produced in LightGbm alone. The only way to prevent this error would be to not assign a parameter to the metric at all.

This warning has not appeared in previous versions of ML.NET and can be prevent by assigning the correct metric based on the objective as was previously done.

To prevent this warning, the changes from this commit were reverted.

@najeeb-kazmi

Copy link
Copy Markdown
Member

If the only reason we are reverting the defaults is this warning that is not affecting training, we should weigh the two options:

  1. Keep the defaults at the expense of having this warning, which might cause confusion for the user.

  2. Revert defaults and avoid the warning at the expense of having inconsistent user experience compared to standalone LightGbm as described in LightGbm default evaluation metrics in ML.NET do not conform to standalone LightGbm #3822 and Change default EvaluationMetric for LightGbm trainers to conform to d… #3859, although this will also cause confusion for the user.

I have a preference for (1) from the point of view of a LightGbm user coming to ML.NET and finding that the defaults are different from what they are used to. That this warning is being printed when the correct LightGbm default i.e. "" is being passed to LightGbm seems to be an issue that should be fixed on the LightGbm side.

@rayankrish
rayankrish merged commit 820d359 into dotnet:masterAug 5, 2019
@ghostghost locked as resolved and limited conversation to collaborators Mar 21, 2022
Sign up for freeto 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.

3 participants

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

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix] - #4007

Merged
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning
Aug 5, 2019
Merged

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix]#4007
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning

Conversation

@rayankrish

Copy link
Copy Markdown

Issue #3965 reported that a warning, "LightGBM] [Warning] Unknown parameter metric=" is produced when the default metric is used. This warning came after this commit which aimed to provide a consistent user experience from an ML.NET implementation of LightGbm with standalone LightGbm. If a user were to set EvaluationMetric = EvaluateMetricType.Default, they might expect that this would set the EvaluationMetric to "" and assigned the metric based on the objective as shown in the LightGbm docs. When the correction was made, this warning began to appear when the metric parameter was set to "". which was also being produced in LightGbm alone. The only way to prevent this error would be to not assign a parameter to the metric at all.

This warning has not appeared in previous versions of ML.NET and can be prevent by assigning the correct metric based on the objective as was previously done.

To prevent this warning, the changes from this commit were reverted.

@najeeb-kazmi

Copy link
Copy Markdown
Member

If the only reason we are reverting the defaults is this warning that is not affecting training, we should weigh the two options:

  1. Keep the defaults at the expense of having this warning, which might cause confusion for the user.

  2. Revert defaults and avoid the warning at the expense of having inconsistent user experience compared to standalone LightGbm as described in LightGbm default evaluation metrics in ML.NET do not conform to standalone LightGbm #3822 and Change default EvaluationMetric for LightGbm trainers to conform to d… #3859, although this will also cause confusion for the user.

I have a preference for (1) from the point of view of a LightGbm user coming to ML.NET and finding that the defaults are different from what they are used to. That this warning is being printed when the correct LightGbm default i.e. "" is being passed to LightGbm seems to be an issue that should be fixed on the LightGbm side.

@rayankrish
rayankrish merged commit 820d359 into dotnet:masterAug 5, 2019
@ghostghost locked as resolved and limited conversation to collaborators Mar 21, 2022
Sign up for freeto 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.

3 participants

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

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix] - #4007

Merged
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning
Aug 5, 2019
Merged

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix]#4007
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning

Conversation

@rayankrish

Copy link
Copy Markdown

Issue #3965 reported that a warning, "LightGBM] [Warning] Unknown parameter metric=" is produced when the default metric is used. This warning came after this commit which aimed to provide a consistent user experience from an ML.NET implementation of LightGbm with standalone LightGbm. If a user were to set EvaluationMetric = EvaluateMetricType.Default, they might expect that this would set the EvaluationMetric to "" and assigned the metric based on the objective as shown in the LightGbm docs. When the correction was made, this warning began to appear when the metric parameter was set to "". which was also being produced in LightGbm alone. The only way to prevent this error would be to not assign a parameter to the metric at all.

This warning has not appeared in previous versions of ML.NET and can be prevent by assigning the correct metric based on the objective as was previously done.

To prevent this warning, the changes from this commit were reverted.

@najeeb-kazmi

Copy link
Copy Markdown
Member

If the only reason we are reverting the defaults is this warning that is not affecting training, we should weigh the two options:

  1. Keep the defaults at the expense of having this warning, which might cause confusion for the user.

  2. Revert defaults and avoid the warning at the expense of having inconsistent user experience compared to standalone LightGbm as described in LightGbm default evaluation metrics in ML.NET do not conform to standalone LightGbm #3822 and Change default EvaluationMetric for LightGbm trainers to conform to d… #3859, although this will also cause confusion for the user.

I have a preference for (1) from the point of view of a LightGbm user coming to ML.NET and finding that the defaults are different from what they are used to. That this warning is being printed when the correct LightGbm default i.e. "" is being passed to LightGbm seems to be an issue that should be fixed on the LightGbm side.

@rayankrish
rayankrish merged commit 820d359 into dotnet:masterAug 5, 2019
@ghostghost locked as resolved and limited conversation to collaborators Mar 21, 2022
Sign up for freeto 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.

3 participants

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

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix] - #4007

Merged
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning
Aug 5, 2019
Merged

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix]#4007
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning

Conversation

@rayankrish

Copy link
Copy Markdown

Issue #3965 reported that a warning, "LightGBM] [Warning] Unknown parameter metric=" is produced when the default metric is used. This warning came after this commit which aimed to provide a consistent user experience from an ML.NET implementation of LightGbm with standalone LightGbm. If a user were to set EvaluationMetric = EvaluateMetricType.Default, they might expect that this would set the EvaluationMetric to "" and assigned the metric based on the objective as shown in the LightGbm docs. When the correction was made, this warning began to appear when the metric parameter was set to "". which was also being produced in LightGbm alone. The only way to prevent this error would be to not assign a parameter to the metric at all.

This warning has not appeared in previous versions of ML.NET and can be prevent by assigning the correct metric based on the objective as was previously done.

To prevent this warning, the changes from this commit were reverted.

@najeeb-kazmi

Copy link
Copy Markdown
Member

If the only reason we are reverting the defaults is this warning that is not affecting training, we should weigh the two options:

  1. Keep the defaults at the expense of having this warning, which might cause confusion for the user.

  2. Revert defaults and avoid the warning at the expense of having inconsistent user experience compared to standalone LightGbm as described in LightGbm default evaluation metrics in ML.NET do not conform to standalone LightGbm #3822 and Change default EvaluationMetric for LightGbm trainers to conform to d… #3859, although this will also cause confusion for the user.

I have a preference for (1) from the point of view of a LightGbm user coming to ML.NET and finding that the defaults are different from what they are used to. That this warning is being printed when the correct LightGbm default i.e. "" is being passed to LightGbm seems to be an issue that should be fixed on the LightGbm side.

@rayankrish
rayankrish merged commit 820d359 into dotnet:masterAug 5, 2019
@ghostghost locked as resolved and limited conversation to collaborators Mar 21, 2022
Sign up for freeto 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.

3 participants

@rayankrish@najeeb-kazmi@sayanshaw24
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix] - #4007

Merged
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning
Aug 5, 2019
Merged

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix]#4007
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning

Conversation

@rayankrish

Copy link
Copy Markdown

Issue #3965 reported that a warning, "LightGBM] [Warning] Unknown parameter metric=" is produced when the default metric is used. This warning came after this commit which aimed to provide a consistent user experience from an ML.NET implementation of LightGbm with standalone LightGbm. If a user were to set EvaluationMetric = EvaluateMetricType.Default, they might expect that this would set the EvaluationMetric to "" and assigned the metric based on the objective as shown in the LightGbm docs. When the correction was made, this warning began to appear when the metric parameter was set to "". which was also being produced in LightGbm alone. The only way to prevent this error would be to not assign a parameter to the metric at all.

This warning has not appeared in previous versions of ML.NET and can be prevent by assigning the correct metric based on the objective as was previously done.

To prevent this warning, the changes from this commit were reverted.

@najeeb-kazmi

Copy link
Copy Markdown
Member

If the only reason we are reverting the defaults is this warning that is not affecting training, we should weigh the two options:

  1. Keep the defaults at the expense of having this warning, which might cause confusion for the user.

  2. Revert defaults and avoid the warning at the expense of having inconsistent user experience compared to standalone LightGbm as described in LightGbm default evaluation metrics in ML.NET do not conform to standalone LightGbm #3822 and Change default EvaluationMetric for LightGbm trainers to conform to d… #3859, although this will also cause confusion for the user.

I have a preference for (1) from the point of view of a LightGbm user coming to ML.NET and finding that the defaults are different from what they are used to. That this warning is being printed when the correct LightGbm default i.e. "" is being passed to LightGbm seems to be an issue that should be fixed on the LightGbm side.

@rayankrish
rayankrish merged commit 820d359 into dotnet:masterAug 5, 2019
@ghostghost locked as resolved and limited conversation to collaborators Mar 21, 2022
Sign up for freeto 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.

3 participants

@rayankrish@najeeb-kazmi@sayanshaw24
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix] - #4007

Merged
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning
Aug 5, 2019
Merged

Stop LightGbm Warning for Default Metric Input [Issue #3965 Fix]#4007
rayankrish merged 2 commits into
dotnet:masterfrom
rayankrish:lgbm-metric-warning

Conversation

@rayankrish

Copy link
Copy Markdown

Issue #3965 reported that a warning, "LightGBM] [Warning] Unknown parameter metric=" is produced when the default metric is used. This warning came after this commit which aimed to provide a consistent user experience from an ML.NET implementation of LightGbm with standalone LightGbm. If a user were to set EvaluationMetric = EvaluateMetricType.Default, they might expect that this would set the EvaluationMetric to "" and assigned the metric based on the objective as shown in the LightGbm docs. When the correction was made, this warning began to appear when the metric parameter was set to "". which was also being produced in LightGbm alone. The only way to prevent this error would be to not assign a parameter to the metric at all.

This warning has not appeared in previous versions of ML.NET and can be prevent by assigning the correct metric based on the objective as was previously done.

To prevent this warning, the changes from this commit were reverted.

@najeeb-kazmi

Copy link
Copy Markdown
Member

If the only reason we are reverting the defaults is this warning that is not affecting training, we should weigh the two options:

  1. Keep the defaults at the expense of having this warning, which might cause confusion for the user.

  2. Revert defaults and avoid the warning at the expense of having inconsistent user experience compared to standalone LightGbm as described in LightGbm default evaluation metrics in ML.NET do not conform to standalone LightGbm #3822 and Change default EvaluationMetric for LightGbm trainers to conform to d… #3859, although this will also cause confusion for the user.

I have a preference for (1) from the point of view of a LightGbm user coming to ML.NET and finding that the defaults are different from what they are used to. That this warning is being printed when the correct LightGbm default i.e. "" is being passed to LightGbm seems to be an issue that should be fixed on the LightGbm side.

@rayankrish
rayankrish merged commit 820d359 into dotnet:masterAug 5, 2019
@ghostghost locked as resolved and limited conversation to collaborators Mar 21, 2022
Sign up for freeto 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.

3 participants

@rayankrish@najeeb-kazmi@sayanshaw24