Add ITrialResultManager for continue training in AutoML - #6335

Merged
LittleLittleCloud merged 15 commits into
dotnet:mainfrom
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Oct 11, 2022
Merged

Add ITrialResultManager for continue training in AutoML#6335
LittleLittleCloud merged 15 commits into
dotnet:mainfrom
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint

Conversation

@LittleLittleCloud

@LittleLittleCloudLittleLittleCloud commented Sep 20, 2022•
edited
Loading

Copy link
Copy Markdown
Member

We are excited to review your PR.

So we can do the best job, please check:

  • There's a descriptive title that will make sense to other developers some time from now.
  • There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  • Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • You have included any necessary tests in the same PR.

Resolved issue

#5736

What's ITrialResultManager

ITrialResultManager is a component used by AutoMLExperiment to save and load TrialResult. The default implementation for ITrialResultManager is CsvTrialResultManager, which saves all trial results in csv format

What's ITrialResultManager used for

Currently, it's used by some tuners (CostFrugalTuner, for example) to recover from previous training, suppose trialResult csv is provided or is available.

How to use it

ITrialResultManager is not directly exposed to public users. User can use SetCheckpoint(string folder) path instead. If a folder is provided via SetCheckpoint, AutoMLExperiment will create that folder if not exists, and configures CsvTrialResultManager to save trial results to folder/trialResults.csv.

@LittleLittleCloudLittleLittleCloud mentioned this pull request Sep 20, 2022
Open
28 tasks
@LittleLittleCloudLittleLittleCloud changed the title Add ITrialResultManagerAdd ITrialResultManager for continue training in AutoMLSep 22, 2022
tarekgh
tarekgh reviewed Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/IMetricManager.cs
{
bool IsMaximize { get; }

string MetricName { get; }

@tarekghtarekghSep 22, 2022

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.

string

I assume there is no app compat concern extending this interface. Extending interfaces on .NET Framework is a breaking change.

tarekgh
tarekgh reviewed Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs Outdated
"loss",
"durationInMilliseconds",
"peakCpu",
"peakMemoryInMegaByte"

@tarekghtarekghSep 22, 2022

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.

nit: please align this block

@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Resolved

tarekgh
tarekgh reviewed Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs Outdated
{
if (!File.Exists(filePath))
{
return new TrialResult[0];

@tarekghtarekghSep 22, 2022

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.

new TrialResult[0];

Array.Empty?

@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Resolved

tarekgh
tarekgh reviewed Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/Flow2.cs Outdated
_step = _stepUpperBound;
}

if (rng != null)

@tarekghtarekghSep 22, 2022

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.

(rng != null)

nit: you don't need this condition I guess.

@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Resolved

tarekgh
tarekgh reviewed Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs Outdated
{
// initialize eci with the estimated cost and always start from pipeline which has lowest cost.
_eci = _pipelineSchemas.ToDictionary(kv => kv, kv => GetEstimatedCostForPipeline(kv, _sweepablePipeline));
_initialized = true;

@tarekghtarekghSep 22, 2022

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.

_initialized

nit: move this line after setting the schema. just in case to be safer for multi-threading scenarios if it is possible.

@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Resolved

tarekgh
tarekgh reviewed Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs Outdated

public void Update(TrialResult result, string schema)
{
_initialized = true;

@tarekghtarekghSep 22, 2022

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.

_initialized

should you set this after finishing the update?

@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Resolved

tarekgh
tarekgh reviewed Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.SearchSpace/SearchSpace.cs Outdated
}

// make sure hash code is greater than 0
return (int)(hash >> 1);

@tarekghtarekghSep 22, 2022

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.

Is it picked up from Java? :-)

You can simplify that like
https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Tuple.cs,69

without using checked blocks at all.

@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

That's cool, thanks!

@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Resolved

tarekgh
tarekgh approved these changes Sep 22, 2022
View reviewed changes

@tarekghtarekgh 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.

Added minor comments and questions. LGTM otherwise.

@codecov

codecovBot commented Sep 23, 2022•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #6335 (b9d7a41) into main (632c373) will decrease coverage by 0.07%.
The diff coverage is 39.47%.

Additional details and impacted files
@@ Coverage Diff @@## main #6335 +/- ##
==========================================
- Coverage 68.58% 68.51% -0.08% 
==========================================
Files 1171 1165 -6 Lines 247964 246142 -1822 Branches 25738 25555 -183 ==========================================
- Hits 170076 168635 -1441 + Misses 71129 70834 -295 + Partials 6759 6673 -86
FlagCoverage Δ
Debug68.51% <39.47%> (-0.08%)⬇️
production62.93% <100.00%> (-0.04%)⬇️
test89.00% <24.59%> (-0.12%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted FilesCoverage Δ
test/Microsoft.ML.AutoML.Tests/AutoFitTests.cs84.94% <ø> (+2.46%)⬆️
...icrosoft.ML.AutoML.Tests/TrialResultManagerTest.cs0.00% <0.00%> (ø)
src/Microsoft.ML.SearchSpace/SearchSpace.cs73.36% <100.00%> (+1.34%)⬆️
src/Microsoft.ML.SearchSpace/Tuner/RandomTuner.cs60.00% <100.00%> (-21.82%)⬇️
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs96.23% <100.00%> (+3.84%)⬆️
.../Microsoft.ML.AutoML.Tests/CostFrugalTunerTests.cs97.20% <100.00%> (+1.30%)⬆️
.../Microsoft.ML.SearchSpace.Tests/SearchSpaceTest.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Maml/MAML.cs24.36% <0.00%> (-2.54%)⬇️
src/Microsoft.Data.Analysis/DataFrame.IO.cs79.88% <0.00%> (-1.28%)⬇️
... and 19 more

@LittleLittleCloud

LittleLittleCloud commented Sep 23, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Sep 23, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@LittleLittleCloud

LittleLittleCloud commented Sep 24, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Sep 24, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@LittleLittleCloud

LittleLittleCloud commented Sep 28, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Sep 28, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@LittleLittleCloud

LittleLittleCloud commented Sep 29, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Sep 29, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@LittleLittleCloud

LittleLittleCloud commented Oct 3, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Oct 3, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@LittleLittleCloud

LittleLittleCloud commented Oct 4, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Oct 4, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@LittleLittleCloud

LittleLittleCloud commented Oct 5, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Oct 5, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@LittleLittleCloud
Update AutoMLExperimentTests.cs
436a05c
disable SweepablePipeline_AutoFit_UCI_Adult_CrossValidation_10_Test
@LittleLittleCloud

LittleLittleCloud commented Oct 5, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Oct 5, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@LittleLittleCloud

LittleLittleCloud commented Oct 6, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Oct 6, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@LittleLittleCloud

LittleLittleCloud commented Oct 7, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Oct 7, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@LittleLittleCloud

LittleLittleCloud commented Oct 7, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Oct 7, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@LittleLittleCloud

LittleLittleCloud commented Oct 10, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@LittleLittleCloud

LittleLittleCloud commented Oct 10, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Oct 10, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@LittleLittleCloud

LittleLittleCloud commented Oct 10, 2022

Copy link
Copy Markdown
MemberAuthor

/azp --help

@azure-pipelines

azure-pipelinesBot commented Oct 10, 2022

Copy link
Copy Markdown
Command '--help' is not supported by Azure Pipelines.

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"

See additional documentation.

@LittleLittleCloud

LittleLittleCloud commented Oct 10, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Oct 10, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@LittleLittleCloud
LittleLittleCloud merged commit cba0162 into dotnet:mainOct 11, 2022
@ghostghost locked as resolved and limited conversation to collaborators Nov 11, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@tarekghtarekghtarekgh approved these changes

Assignees

@LittleLittleCloudLittleLittleCloud

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@LittleLittleCloud@tarekgh@dakersnar
, '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
Add ITrialResultManager for continue training in AutoML - #6335
Merged
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Oct 11, 2022
Merged
Add ITrialResultManager for continue training in AutoML#6335
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Sep 20, 2022•

edited

Loading

Copy link


Copy Markdown

Member
We are excited to review your PR.
So we can do the best job, please check:
  •  There's a descriptive title that will make sense to other developers some time from now.
  •  There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  •  Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  •  You have included any necessary tests in the same PR.
Resolved issue
#5736
What's ITrialResultManager
ITrialResultManager is a component used by AutoMLExperiment to save and load TrialResult. The default implementation for ITrialResultManager is CsvTrialResultManager, which saves all trial results in csv format
What's ITrialResultManager used for
Currently, it's used by some tuners (CostFrugalTuner, for example) to recover from previous training, suppose trialResult csv is provided or is available.
How to use it
ITrialResultManager is not directly exposed to public users. User can use SetCheckpoint(string folder) path instead. If a folder is provided via SetCheckpoint, AutoMLExperiment will create that folder if not exists, and configures CsvTrialResultManager to save trial results to folder/trialResults.csv.
@LittleLittleCloudLittleLittleCloud
mentioned this pull request
Sep 20, 2022

Open

28 tasks
@LittleLittleCloudLittleLittleCloud
changed the title
Add ITrialResultManagerAdd ITrialResultManager for continue training in AutoMLSep 22, 2022
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/IMetricManager.cs
 {
 bool IsMaximize { get; }

 string MetricName { get; }
@tarekghtarekghSep 22, 2022

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.

string 
I assume there is no app compat concern extending this interface. Extending interfaces on .NET Framework is a breaking change.
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 "loss",
 "durationInMilliseconds",
 "peakCpu",
 "peakMemoryInMegaByte"
@tarekghtarekghSep 22, 2022

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.

nit: please align this block
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 {
 if (!File.Exists(filePath))
 {
 return new TrialResult[0];
@tarekghtarekghSep 22, 2022

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.

new TrialResult[0]; 
Array.Empty?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/Flow2.cs
Outdated

 _step = _stepUpperBound;
 }

 if (rng != null)
@tarekghtarekghSep 22, 2022

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.

(rng != null) 
nit: you don't need this condition I guess.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated

 {
 // initialize eci with the estimated cost and always start from pipeline which has lowest cost.
 _eci = _pipelineSchemas.ToDictionary(kv => kv, kv => GetEstimatedCostForPipeline(kv, _sweepablePipeline));
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
nit: move this line after setting the schema. just in case to be safer for multi-threading scenarios if it is possible.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated


 public void Update(TrialResult result, string schema)
 {
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
should you set this after finishing the update?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.SearchSpace/SearchSpace.cs
Outdated

 }

 // make sure hash code is greater than 0
 return (int)(hash >> 1);
@tarekghtarekghSep 22, 2022

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.

Is it picked up from Java? :-)
You can simplify that like
https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Tuple.cs,69
without using checked blocks at all.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

That's cool, thanks!
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
approved these changes
Sep 22, 2022
View reviewed changes
@tarekghtarekgh
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.

Added minor comments and questions. LGTM otherwise.
@codecov
codecovBot
commented
Sep 23, 2022•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6335 (b9d7a41) into main (632c373) will decrease coverage by 0.07%.

The diff coverage is 39.47%.
Additional details and impacted files
@@ Coverage Diff @@## main #6335 +/- ##
==========================================
- Coverage 68.58% 68.51% -0.08% 
==========================================
Files 1171 1165 -6 Lines 247964 246142 -1822 Branches 25738 25555 -183 ==========================================
- Hits 170076 168635 -1441 + Misses 71129 70834 -295 + Partials 6759 6673 -86 
FlagCoverage Δ
Debug68.51% <39.47%> (-0.08%)⬇️
production62.93% <100.00%> (-0.04%)⬇️
test89.00% <24.59%> (-0.12%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
test/Microsoft.ML.AutoML.Tests/AutoFitTests.cs84.94% <ø> (+2.46%)⬆️
...icrosoft.ML.AutoML.Tests/TrialResultManagerTest.cs0.00% <0.00%> (ø)
src/Microsoft.ML.SearchSpace/SearchSpace.cs73.36% <100.00%> (+1.34%)⬆️
src/Microsoft.ML.SearchSpace/Tuner/RandomTuner.cs60.00% <100.00%> (-21.82%)⬇️
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs96.23% <100.00%> (+3.84%)⬆️
.../Microsoft.ML.AutoML.Tests/CostFrugalTunerTests.cs97.20% <100.00%> (+1.30%)⬆️
.../Microsoft.ML.SearchSpace.Tests/SearchSpaceTest.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Maml/MAML.cs24.36% <0.00%> (-2.54%)⬇️
src/Microsoft.Data.Analysis/DataFrame.IO.cs79.88% <0.00%> (-1.28%)⬇️
... and 19 more
@LittleLittleCloud
LittleLittleCloud
commented
Sep 23, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 23, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 24, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 24, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 28, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 28, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 29, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 29, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 4, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
Update AutoMLExperimentTests.cs
436a05c
disable SweepablePipeline_AutoFit_UCI_Adult_CrossValidation_10_Test
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 6, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 6, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp --help
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Command '--help' is not supported by Azure Pipelines.

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"


See additional documentation.
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
merged commit cba0162
into
dotnet:mainOct 11, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Nov 11, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

Assignees

@LittleLittleCloudLittleLittleCloud

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@LittleLittleCloud@tarekgh@dakersnar

, '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
Add ITrialResultManager for continue training in AutoML - #6335
Merged
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Oct 11, 2022
Merged
Add ITrialResultManager for continue training in AutoML#6335
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Sep 20, 2022•

edited

Loading

Copy link


Copy Markdown

Member
We are excited to review your PR.
So we can do the best job, please check:
  •  There's a descriptive title that will make sense to other developers some time from now.
  •  There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  •  Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  •  You have included any necessary tests in the same PR.
Resolved issue
#5736
What's ITrialResultManager
ITrialResultManager is a component used by AutoMLExperiment to save and load TrialResult. The default implementation for ITrialResultManager is CsvTrialResultManager, which saves all trial results in csv format
What's ITrialResultManager used for
Currently, it's used by some tuners (CostFrugalTuner, for example) to recover from previous training, suppose trialResult csv is provided or is available.
How to use it
ITrialResultManager is not directly exposed to public users. User can use SetCheckpoint(string folder) path instead. If a folder is provided via SetCheckpoint, AutoMLExperiment will create that folder if not exists, and configures CsvTrialResultManager to save trial results to folder/trialResults.csv.
@LittleLittleCloudLittleLittleCloud
mentioned this pull request
Sep 20, 2022

Open

28 tasks
@LittleLittleCloudLittleLittleCloud
changed the title
Add ITrialResultManagerAdd ITrialResultManager for continue training in AutoMLSep 22, 2022
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/IMetricManager.cs
 {
 bool IsMaximize { get; }

 string MetricName { get; }
@tarekghtarekghSep 22, 2022

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.

string 
I assume there is no app compat concern extending this interface. Extending interfaces on .NET Framework is a breaking change.
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 "loss",
 "durationInMilliseconds",
 "peakCpu",
 "peakMemoryInMegaByte"
@tarekghtarekghSep 22, 2022

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.

nit: please align this block
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 {
 if (!File.Exists(filePath))
 {
 return new TrialResult[0];
@tarekghtarekghSep 22, 2022

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.

new TrialResult[0]; 
Array.Empty?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/Flow2.cs
Outdated

 _step = _stepUpperBound;
 }

 if (rng != null)
@tarekghtarekghSep 22, 2022

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.

(rng != null) 
nit: you don't need this condition I guess.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated

 {
 // initialize eci with the estimated cost and always start from pipeline which has lowest cost.
 _eci = _pipelineSchemas.ToDictionary(kv => kv, kv => GetEstimatedCostForPipeline(kv, _sweepablePipeline));
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
nit: move this line after setting the schema. just in case to be safer for multi-threading scenarios if it is possible.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated


 public void Update(TrialResult result, string schema)
 {
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
should you set this after finishing the update?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.SearchSpace/SearchSpace.cs
Outdated

 }

 // make sure hash code is greater than 0
 return (int)(hash >> 1);
@tarekghtarekghSep 22, 2022

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.

Is it picked up from Java? :-)
You can simplify that like
https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Tuple.cs,69
without using checked blocks at all.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

That's cool, thanks!
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
approved these changes
Sep 22, 2022
View reviewed changes
@tarekghtarekgh
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.

Added minor comments and questions. LGTM otherwise.
@codecov
codecovBot
commented
Sep 23, 2022•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6335 (b9d7a41) into main (632c373) will decrease coverage by 0.07%.

The diff coverage is 39.47%.
Additional details and impacted files
@@ Coverage Diff @@## main #6335 +/- ##
==========================================
- Coverage 68.58% 68.51% -0.08% 
==========================================
Files 1171 1165 -6 Lines 247964 246142 -1822 Branches 25738 25555 -183 ==========================================
- Hits 170076 168635 -1441 + Misses 71129 70834 -295 + Partials 6759 6673 -86 
FlagCoverage Δ
Debug68.51% <39.47%> (-0.08%)⬇️
production62.93% <100.00%> (-0.04%)⬇️
test89.00% <24.59%> (-0.12%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
test/Microsoft.ML.AutoML.Tests/AutoFitTests.cs84.94% <ø> (+2.46%)⬆️
...icrosoft.ML.AutoML.Tests/TrialResultManagerTest.cs0.00% <0.00%> (ø)
src/Microsoft.ML.SearchSpace/SearchSpace.cs73.36% <100.00%> (+1.34%)⬆️
src/Microsoft.ML.SearchSpace/Tuner/RandomTuner.cs60.00% <100.00%> (-21.82%)⬇️
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs96.23% <100.00%> (+3.84%)⬆️
.../Microsoft.ML.AutoML.Tests/CostFrugalTunerTests.cs97.20% <100.00%> (+1.30%)⬆️
.../Microsoft.ML.SearchSpace.Tests/SearchSpaceTest.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Maml/MAML.cs24.36% <0.00%> (-2.54%)⬇️
src/Microsoft.Data.Analysis/DataFrame.IO.cs79.88% <0.00%> (-1.28%)⬇️
... and 19 more
@LittleLittleCloud
LittleLittleCloud
commented
Sep 23, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 23, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 24, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 24, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 28, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 28, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 29, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 29, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 4, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
Update AutoMLExperimentTests.cs
436a05c
disable SweepablePipeline_AutoFit_UCI_Adult_CrossValidation_10_Test
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 6, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 6, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp --help
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Command '--help' is not supported by Azure Pipelines.

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"


See additional documentation.
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
merged commit cba0162
into
dotnet:mainOct 11, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Nov 11, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

Assignees

@LittleLittleCloudLittleLittleCloud

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@LittleLittleCloud@tarekgh@dakersnar

, '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
Add ITrialResultManager for continue training in AutoML - #6335
Merged
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Oct 11, 2022
Merged
Add ITrialResultManager for continue training in AutoML#6335
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Sep 20, 2022•

edited

Loading

Copy link


Copy Markdown

Member
We are excited to review your PR.
So we can do the best job, please check:
  •  There's a descriptive title that will make sense to other developers some time from now.
  •  There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  •  Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  •  You have included any necessary tests in the same PR.
Resolved issue
#5736
What's ITrialResultManager
ITrialResultManager is a component used by AutoMLExperiment to save and load TrialResult. The default implementation for ITrialResultManager is CsvTrialResultManager, which saves all trial results in csv format
What's ITrialResultManager used for
Currently, it's used by some tuners (CostFrugalTuner, for example) to recover from previous training, suppose trialResult csv is provided or is available.
How to use it
ITrialResultManager is not directly exposed to public users. User can use SetCheckpoint(string folder) path instead. If a folder is provided via SetCheckpoint, AutoMLExperiment will create that folder if not exists, and configures CsvTrialResultManager to save trial results to folder/trialResults.csv.
@LittleLittleCloudLittleLittleCloud
mentioned this pull request
Sep 20, 2022

Open

28 tasks
@LittleLittleCloudLittleLittleCloud
changed the title
Add ITrialResultManagerAdd ITrialResultManager for continue training in AutoMLSep 22, 2022
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/IMetricManager.cs
 {
 bool IsMaximize { get; }

 string MetricName { get; }
@tarekghtarekghSep 22, 2022

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.

string 
I assume there is no app compat concern extending this interface. Extending interfaces on .NET Framework is a breaking change.
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 "loss",
 "durationInMilliseconds",
 "peakCpu",
 "peakMemoryInMegaByte"
@tarekghtarekghSep 22, 2022

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.

nit: please align this block
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 {
 if (!File.Exists(filePath))
 {
 return new TrialResult[0];
@tarekghtarekghSep 22, 2022

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.

new TrialResult[0]; 
Array.Empty?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/Flow2.cs
Outdated

 _step = _stepUpperBound;
 }

 if (rng != null)
@tarekghtarekghSep 22, 2022

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.

(rng != null) 
nit: you don't need this condition I guess.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated

 {
 // initialize eci with the estimated cost and always start from pipeline which has lowest cost.
 _eci = _pipelineSchemas.ToDictionary(kv => kv, kv => GetEstimatedCostForPipeline(kv, _sweepablePipeline));
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
nit: move this line after setting the schema. just in case to be safer for multi-threading scenarios if it is possible.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated


 public void Update(TrialResult result, string schema)
 {
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
should you set this after finishing the update?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.SearchSpace/SearchSpace.cs
Outdated

 }

 // make sure hash code is greater than 0
 return (int)(hash >> 1);
@tarekghtarekghSep 22, 2022

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.

Is it picked up from Java? :-)
You can simplify that like
https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Tuple.cs,69
without using checked blocks at all.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

That's cool, thanks!
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
approved these changes
Sep 22, 2022
View reviewed changes
@tarekghtarekgh
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.

Added minor comments and questions. LGTM otherwise.
@codecov
codecovBot
commented
Sep 23, 2022•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6335 (b9d7a41) into main (632c373) will decrease coverage by 0.07%.

The diff coverage is 39.47%.
Additional details and impacted files
@@ Coverage Diff @@## main #6335 +/- ##
==========================================
- Coverage 68.58% 68.51% -0.08% 
==========================================
Files 1171 1165 -6 Lines 247964 246142 -1822 Branches 25738 25555 -183 ==========================================
- Hits 170076 168635 -1441 + Misses 71129 70834 -295 + Partials 6759 6673 -86 
FlagCoverage Δ
Debug68.51% <39.47%> (-0.08%)⬇️
production62.93% <100.00%> (-0.04%)⬇️
test89.00% <24.59%> (-0.12%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
test/Microsoft.ML.AutoML.Tests/AutoFitTests.cs84.94% <ø> (+2.46%)⬆️
...icrosoft.ML.AutoML.Tests/TrialResultManagerTest.cs0.00% <0.00%> (ø)
src/Microsoft.ML.SearchSpace/SearchSpace.cs73.36% <100.00%> (+1.34%)⬆️
src/Microsoft.ML.SearchSpace/Tuner/RandomTuner.cs60.00% <100.00%> (-21.82%)⬇️
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs96.23% <100.00%> (+3.84%)⬆️
.../Microsoft.ML.AutoML.Tests/CostFrugalTunerTests.cs97.20% <100.00%> (+1.30%)⬆️
.../Microsoft.ML.SearchSpace.Tests/SearchSpaceTest.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Maml/MAML.cs24.36% <0.00%> (-2.54%)⬇️
src/Microsoft.Data.Analysis/DataFrame.IO.cs79.88% <0.00%> (-1.28%)⬇️
... and 19 more
@LittleLittleCloud
LittleLittleCloud
commented
Sep 23, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 23, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 24, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 24, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 28, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 28, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 29, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 29, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 4, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
Update AutoMLExperimentTests.cs
436a05c
disable SweepablePipeline_AutoFit_UCI_Adult_CrossValidation_10_Test
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 6, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 6, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp --help
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Command '--help' is not supported by Azure Pipelines.

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"


See additional documentation.
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
merged commit cba0162
into
dotnet:mainOct 11, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Nov 11, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

Assignees

@LittleLittleCloudLittleLittleCloud

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@LittleLittleCloud@tarekgh@dakersnar

, '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
Add ITrialResultManager for continue training in AutoML - #6335
Merged
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Oct 11, 2022
Merged
Add ITrialResultManager for continue training in AutoML#6335
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Sep 20, 2022•

edited

Loading

Copy link


Copy Markdown

Member
We are excited to review your PR.
So we can do the best job, please check:
  •  There's a descriptive title that will make sense to other developers some time from now.
  •  There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  •  Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  •  You have included any necessary tests in the same PR.
Resolved issue
#5736
What's ITrialResultManager
ITrialResultManager is a component used by AutoMLExperiment to save and load TrialResult. The default implementation for ITrialResultManager is CsvTrialResultManager, which saves all trial results in csv format
What's ITrialResultManager used for
Currently, it's used by some tuners (CostFrugalTuner, for example) to recover from previous training, suppose trialResult csv is provided or is available.
How to use it
ITrialResultManager is not directly exposed to public users. User can use SetCheckpoint(string folder) path instead. If a folder is provided via SetCheckpoint, AutoMLExperiment will create that folder if not exists, and configures CsvTrialResultManager to save trial results to folder/trialResults.csv.
@LittleLittleCloudLittleLittleCloud
mentioned this pull request
Sep 20, 2022

Open

28 tasks
@LittleLittleCloudLittleLittleCloud
changed the title
Add ITrialResultManagerAdd ITrialResultManager for continue training in AutoMLSep 22, 2022
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/IMetricManager.cs
 {
 bool IsMaximize { get; }

 string MetricName { get; }
@tarekghtarekghSep 22, 2022

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.

string 
I assume there is no app compat concern extending this interface. Extending interfaces on .NET Framework is a breaking change.
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 "loss",
 "durationInMilliseconds",
 "peakCpu",
 "peakMemoryInMegaByte"
@tarekghtarekghSep 22, 2022

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.

nit: please align this block
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 {
 if (!File.Exists(filePath))
 {
 return new TrialResult[0];
@tarekghtarekghSep 22, 2022

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.

new TrialResult[0]; 
Array.Empty?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/Flow2.cs
Outdated

 _step = _stepUpperBound;
 }

 if (rng != null)
@tarekghtarekghSep 22, 2022

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.

(rng != null) 
nit: you don't need this condition I guess.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated

 {
 // initialize eci with the estimated cost and always start from pipeline which has lowest cost.
 _eci = _pipelineSchemas.ToDictionary(kv => kv, kv => GetEstimatedCostForPipeline(kv, _sweepablePipeline));
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
nit: move this line after setting the schema. just in case to be safer for multi-threading scenarios if it is possible.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated


 public void Update(TrialResult result, string schema)
 {
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
should you set this after finishing the update?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.SearchSpace/SearchSpace.cs
Outdated

 }

 // make sure hash code is greater than 0
 return (int)(hash >> 1);
@tarekghtarekghSep 22, 2022

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.

Is it picked up from Java? :-)
You can simplify that like
https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Tuple.cs,69
without using checked blocks at all.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

That's cool, thanks!
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
approved these changes
Sep 22, 2022
View reviewed changes
@tarekghtarekgh
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.

Added minor comments and questions. LGTM otherwise.
@codecov
codecovBot
commented
Sep 23, 2022•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6335 (b9d7a41) into main (632c373) will decrease coverage by 0.07%.

The diff coverage is 39.47%.
Additional details and impacted files
@@ Coverage Diff @@## main #6335 +/- ##
==========================================
- Coverage 68.58% 68.51% -0.08% 
==========================================
Files 1171 1165 -6 Lines 247964 246142 -1822 Branches 25738 25555 -183 ==========================================
- Hits 170076 168635 -1441 + Misses 71129 70834 -295 + Partials 6759 6673 -86 
FlagCoverage Δ
Debug68.51% <39.47%> (-0.08%)⬇️
production62.93% <100.00%> (-0.04%)⬇️
test89.00% <24.59%> (-0.12%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
test/Microsoft.ML.AutoML.Tests/AutoFitTests.cs84.94% <ø> (+2.46%)⬆️
...icrosoft.ML.AutoML.Tests/TrialResultManagerTest.cs0.00% <0.00%> (ø)
src/Microsoft.ML.SearchSpace/SearchSpace.cs73.36% <100.00%> (+1.34%)⬆️
src/Microsoft.ML.SearchSpace/Tuner/RandomTuner.cs60.00% <100.00%> (-21.82%)⬇️
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs96.23% <100.00%> (+3.84%)⬆️
.../Microsoft.ML.AutoML.Tests/CostFrugalTunerTests.cs97.20% <100.00%> (+1.30%)⬆️
.../Microsoft.ML.SearchSpace.Tests/SearchSpaceTest.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Maml/MAML.cs24.36% <0.00%> (-2.54%)⬇️
src/Microsoft.Data.Analysis/DataFrame.IO.cs79.88% <0.00%> (-1.28%)⬇️
... and 19 more
@LittleLittleCloud
LittleLittleCloud
commented
Sep 23, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 23, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 24, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 24, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 28, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 28, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 29, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 29, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 4, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
Update AutoMLExperimentTests.cs
436a05c
disable SweepablePipeline_AutoFit_UCI_Adult_CrossValidation_10_Test
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 6, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 6, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp --help
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Command '--help' is not supported by Azure Pipelines.

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"


See additional documentation.
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
merged commit cba0162
into
dotnet:mainOct 11, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Nov 11, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

Assignees

@LittleLittleCloudLittleLittleCloud

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@LittleLittleCloud@tarekgh@dakersnar

, '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
Add ITrialResultManager for continue training in AutoML - #6335
Merged
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Oct 11, 2022
Merged
Add ITrialResultManager for continue training in AutoML#6335
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Sep 20, 2022•

edited

Loading

Copy link


Copy Markdown

Member
We are excited to review your PR.
So we can do the best job, please check:
  •  There's a descriptive title that will make sense to other developers some time from now.
  •  There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  •  Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  •  You have included any necessary tests in the same PR.
Resolved issue
#5736
What's ITrialResultManager
ITrialResultManager is a component used by AutoMLExperiment to save and load TrialResult. The default implementation for ITrialResultManager is CsvTrialResultManager, which saves all trial results in csv format
What's ITrialResultManager used for
Currently, it's used by some tuners (CostFrugalTuner, for example) to recover from previous training, suppose trialResult csv is provided or is available.
How to use it
ITrialResultManager is not directly exposed to public users. User can use SetCheckpoint(string folder) path instead. If a folder is provided via SetCheckpoint, AutoMLExperiment will create that folder if not exists, and configures CsvTrialResultManager to save trial results to folder/trialResults.csv.
@LittleLittleCloudLittleLittleCloud
mentioned this pull request
Sep 20, 2022

Open

28 tasks
@LittleLittleCloudLittleLittleCloud
changed the title
Add ITrialResultManagerAdd ITrialResultManager for continue training in AutoMLSep 22, 2022
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/IMetricManager.cs
 {
 bool IsMaximize { get; }

 string MetricName { get; }
@tarekghtarekghSep 22, 2022

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.

string 
I assume there is no app compat concern extending this interface. Extending interfaces on .NET Framework is a breaking change.
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 "loss",
 "durationInMilliseconds",
 "peakCpu",
 "peakMemoryInMegaByte"
@tarekghtarekghSep 22, 2022

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.

nit: please align this block
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 {
 if (!File.Exists(filePath))
 {
 return new TrialResult[0];
@tarekghtarekghSep 22, 2022

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.

new TrialResult[0]; 
Array.Empty?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/Flow2.cs
Outdated

 _step = _stepUpperBound;
 }

 if (rng != null)
@tarekghtarekghSep 22, 2022

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.

(rng != null) 
nit: you don't need this condition I guess.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated

 {
 // initialize eci with the estimated cost and always start from pipeline which has lowest cost.
 _eci = _pipelineSchemas.ToDictionary(kv => kv, kv => GetEstimatedCostForPipeline(kv, _sweepablePipeline));
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
nit: move this line after setting the schema. just in case to be safer for multi-threading scenarios if it is possible.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated


 public void Update(TrialResult result, string schema)
 {
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
should you set this after finishing the update?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.SearchSpace/SearchSpace.cs
Outdated

 }

 // make sure hash code is greater than 0
 return (int)(hash >> 1);
@tarekghtarekghSep 22, 2022

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.

Is it picked up from Java? :-)
You can simplify that like
https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Tuple.cs,69
without using checked blocks at all.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

That's cool, thanks!
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
approved these changes
Sep 22, 2022
View reviewed changes
@tarekghtarekgh
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.

Added minor comments and questions. LGTM otherwise.
@codecov
codecovBot
commented
Sep 23, 2022•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6335 (b9d7a41) into main (632c373) will decrease coverage by 0.07%.

The diff coverage is 39.47%.
Additional details and impacted files
@@ Coverage Diff @@## main #6335 +/- ##
==========================================
- Coverage 68.58% 68.51% -0.08% 
==========================================
Files 1171 1165 -6 Lines 247964 246142 -1822 Branches 25738 25555 -183 ==========================================
- Hits 170076 168635 -1441 + Misses 71129 70834 -295 + Partials 6759 6673 -86 
FlagCoverage Δ
Debug68.51% <39.47%> (-0.08%)⬇️
production62.93% <100.00%> (-0.04%)⬇️
test89.00% <24.59%> (-0.12%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
test/Microsoft.ML.AutoML.Tests/AutoFitTests.cs84.94% <ø> (+2.46%)⬆️
...icrosoft.ML.AutoML.Tests/TrialResultManagerTest.cs0.00% <0.00%> (ø)
src/Microsoft.ML.SearchSpace/SearchSpace.cs73.36% <100.00%> (+1.34%)⬆️
src/Microsoft.ML.SearchSpace/Tuner/RandomTuner.cs60.00% <100.00%> (-21.82%)⬇️
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs96.23% <100.00%> (+3.84%)⬆️
.../Microsoft.ML.AutoML.Tests/CostFrugalTunerTests.cs97.20% <100.00%> (+1.30%)⬆️
.../Microsoft.ML.SearchSpace.Tests/SearchSpaceTest.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Maml/MAML.cs24.36% <0.00%> (-2.54%)⬇️
src/Microsoft.Data.Analysis/DataFrame.IO.cs79.88% <0.00%> (-1.28%)⬇️
... and 19 more
@LittleLittleCloud
LittleLittleCloud
commented
Sep 23, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 23, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 24, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 24, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 28, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 28, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 29, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 29, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 4, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
Update AutoMLExperimentTests.cs
436a05c
disable SweepablePipeline_AutoFit_UCI_Adult_CrossValidation_10_Test
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 6, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 6, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp --help
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Command '--help' is not supported by Azure Pipelines.

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"


See additional documentation.
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
merged commit cba0162
into
dotnet:mainOct 11, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Nov 11, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

Assignees

@LittleLittleCloudLittleLittleCloud

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@LittleLittleCloud@tarekgh@dakersnar

, '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
Add ITrialResultManager for continue training in AutoML - #6335
Merged
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Oct 11, 2022
Merged
Add ITrialResultManager for continue training in AutoML#6335
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Sep 20, 2022•

edited

Loading

Copy link


Copy Markdown

Member
We are excited to review your PR.
So we can do the best job, please check:
  •  There's a descriptive title that will make sense to other developers some time from now.
  •  There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  •  Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  •  You have included any necessary tests in the same PR.
Resolved issue
#5736
What's ITrialResultManager
ITrialResultManager is a component used by AutoMLExperiment to save and load TrialResult. The default implementation for ITrialResultManager is CsvTrialResultManager, which saves all trial results in csv format
What's ITrialResultManager used for
Currently, it's used by some tuners (CostFrugalTuner, for example) to recover from previous training, suppose trialResult csv is provided or is available.
How to use it
ITrialResultManager is not directly exposed to public users. User can use SetCheckpoint(string folder) path instead. If a folder is provided via SetCheckpoint, AutoMLExperiment will create that folder if not exists, and configures CsvTrialResultManager to save trial results to folder/trialResults.csv.
@LittleLittleCloudLittleLittleCloud
mentioned this pull request
Sep 20, 2022

Open

28 tasks
@LittleLittleCloudLittleLittleCloud
changed the title
Add ITrialResultManagerAdd ITrialResultManager for continue training in AutoMLSep 22, 2022
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/IMetricManager.cs
 {
 bool IsMaximize { get; }

 string MetricName { get; }
@tarekghtarekghSep 22, 2022

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.

string 
I assume there is no app compat concern extending this interface. Extending interfaces on .NET Framework is a breaking change.
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 "loss",
 "durationInMilliseconds",
 "peakCpu",
 "peakMemoryInMegaByte"
@tarekghtarekghSep 22, 2022

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.

nit: please align this block
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 {
 if (!File.Exists(filePath))
 {
 return new TrialResult[0];
@tarekghtarekghSep 22, 2022

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.

new TrialResult[0]; 
Array.Empty?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/Flow2.cs
Outdated

 _step = _stepUpperBound;
 }

 if (rng != null)
@tarekghtarekghSep 22, 2022

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.

(rng != null) 
nit: you don't need this condition I guess.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated

 {
 // initialize eci with the estimated cost and always start from pipeline which has lowest cost.
 _eci = _pipelineSchemas.ToDictionary(kv => kv, kv => GetEstimatedCostForPipeline(kv, _sweepablePipeline));
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
nit: move this line after setting the schema. just in case to be safer for multi-threading scenarios if it is possible.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated


 public void Update(TrialResult result, string schema)
 {
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
should you set this after finishing the update?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.SearchSpace/SearchSpace.cs
Outdated

 }

 // make sure hash code is greater than 0
 return (int)(hash >> 1);
@tarekghtarekghSep 22, 2022

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.

Is it picked up from Java? :-)
You can simplify that like
https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Tuple.cs,69
without using checked blocks at all.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

That's cool, thanks!
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
approved these changes
Sep 22, 2022
View reviewed changes
@tarekghtarekgh
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.

Added minor comments and questions. LGTM otherwise.
@codecov
codecovBot
commented
Sep 23, 2022•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6335 (b9d7a41) into main (632c373) will decrease coverage by 0.07%.

The diff coverage is 39.47%.
Additional details and impacted files
@@ Coverage Diff @@## main #6335 +/- ##
==========================================
- Coverage 68.58% 68.51% -0.08% 
==========================================
Files 1171 1165 -6 Lines 247964 246142 -1822 Branches 25738 25555 -183 ==========================================
- Hits 170076 168635 -1441 + Misses 71129 70834 -295 + Partials 6759 6673 -86 
FlagCoverage Δ
Debug68.51% <39.47%> (-0.08%)⬇️
production62.93% <100.00%> (-0.04%)⬇️
test89.00% <24.59%> (-0.12%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
test/Microsoft.ML.AutoML.Tests/AutoFitTests.cs84.94% <ø> (+2.46%)⬆️
...icrosoft.ML.AutoML.Tests/TrialResultManagerTest.cs0.00% <0.00%> (ø)
src/Microsoft.ML.SearchSpace/SearchSpace.cs73.36% <100.00%> (+1.34%)⬆️
src/Microsoft.ML.SearchSpace/Tuner/RandomTuner.cs60.00% <100.00%> (-21.82%)⬇️
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs96.23% <100.00%> (+3.84%)⬆️
.../Microsoft.ML.AutoML.Tests/CostFrugalTunerTests.cs97.20% <100.00%> (+1.30%)⬆️
.../Microsoft.ML.SearchSpace.Tests/SearchSpaceTest.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Maml/MAML.cs24.36% <0.00%> (-2.54%)⬇️
src/Microsoft.Data.Analysis/DataFrame.IO.cs79.88% <0.00%> (-1.28%)⬇️
... and 19 more
@LittleLittleCloud
LittleLittleCloud
commented
Sep 23, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 23, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 24, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 24, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 28, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 28, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 29, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 29, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 4, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
Update AutoMLExperimentTests.cs
436a05c
disable SweepablePipeline_AutoFit_UCI_Adult_CrossValidation_10_Test
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 6, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 6, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp --help
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Command '--help' is not supported by Azure Pipelines.

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"


See additional documentation.
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
merged commit cba0162
into
dotnet:mainOct 11, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Nov 11, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

Assignees

@LittleLittleCloudLittleLittleCloud

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@LittleLittleCloud@tarekgh@dakersnar

, '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
Add ITrialResultManager for continue training in AutoML - #6335
Merged
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Oct 11, 2022
Merged
Add ITrialResultManager for continue training in AutoML#6335
LittleLittleCloud merged 15 commits into
dotnet:mainfrom 
LittleLittleCloud:u/xiaoyun/autoMLCheckPoint
Conversation
@LittleLittleCloud
@LittleLittleCloudLittleLittleCloud
commented
Sep 20, 2022•

edited

Loading

Copy link


Copy Markdown

Member
We are excited to review your PR.
So we can do the best job, please check:
  •  There's a descriptive title that will make sense to other developers some time from now.
  •  There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  •  Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  •  You have included any necessary tests in the same PR.
Resolved issue
#5736
What's ITrialResultManager
ITrialResultManager is a component used by AutoMLExperiment to save and load TrialResult. The default implementation for ITrialResultManager is CsvTrialResultManager, which saves all trial results in csv format
What's ITrialResultManager used for
Currently, it's used by some tuners (CostFrugalTuner, for example) to recover from previous training, suppose trialResult csv is provided or is available.
How to use it
ITrialResultManager is not directly exposed to public users. User can use SetCheckpoint(string folder) path instead. If a folder is provided via SetCheckpoint, AutoMLExperiment will create that folder if not exists, and configures CsvTrialResultManager to save trial results to folder/trialResults.csv.
@LittleLittleCloudLittleLittleCloud
mentioned this pull request
Sep 20, 2022

Open

28 tasks
@LittleLittleCloudLittleLittleCloud
changed the title
Add ITrialResultManagerAdd ITrialResultManager for continue training in AutoMLSep 22, 2022
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/IMetricManager.cs
 {
 bool IsMaximize { get; }

 string MetricName { get; }
@tarekghtarekghSep 22, 2022

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.

string 
I assume there is no app compat concern extending this interface. Extending interfaces on .NET Framework is a breaking change.
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 "loss",
 "durationInMilliseconds",
 "peakCpu",
 "peakMemoryInMegaByte"
@tarekghtarekghSep 22, 2022

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.

nit: please align this block
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/AutoMLExperiment/ITrialResultManager.cs
Outdated

 {
 if (!File.Exists(filePath))
 {
 return new TrialResult[0];
@tarekghtarekghSep 22, 2022

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.

new TrialResult[0]; 
Array.Empty?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/Flow2.cs
Outdated

 _step = _stepUpperBound;
 }

 if (rng != null)
@tarekghtarekghSep 22, 2022

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.

(rng != null) 
nit: you don't need this condition I guess.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated

 {
 // initialize eci with the estimated cost and always start from pipeline which has lowest cost.
 _eci = _pipelineSchemas.ToDictionary(kv => kv, kv => GetEstimatedCostForPipeline(kv, _sweepablePipeline));
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
nit: move this line after setting the schema. just in case to be safer for multi-threading scenarios if it is possible.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.AutoML/Tuner/PipelineProposer.cs
Outdated


 public void Update(TrialResult result, string schema)
 {
 _initialized = true;
@tarekghtarekghSep 22, 2022

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.

_initialized 
should you set this after finishing the update?
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
reviewed
Sep 22, 2022
View reviewed changes
Comment threadsrc/Microsoft.ML.SearchSpace/SearchSpace.cs
Outdated

 }

 // make sure hash code is greater than 0
 return (int)(hash >> 1);
@tarekghtarekghSep 22, 2022

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.

Is it picked up from Java? :-)
You can simplify that like
https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Tuple.cs,69
without using checked blocks at all.
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

That's cool, thanks!
@LittleLittleCloudLittleLittleCloudSep 23, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Resolved
tarekgh
tarekgh
approved these changes
Sep 22, 2022
View reviewed changes
@tarekghtarekgh
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.

Added minor comments and questions. LGTM otherwise.
@codecov
codecovBot
commented
Sep 23, 2022•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6335 (b9d7a41) into main (632c373) will decrease coverage by 0.07%.

The diff coverage is 39.47%.
Additional details and impacted files
@@ Coverage Diff @@## main #6335 +/- ##
==========================================
- Coverage 68.58% 68.51% -0.08% 
==========================================
Files 1171 1165 -6 Lines 247964 246142 -1822 Branches 25738 25555 -183 ==========================================
- Hits 170076 168635 -1441 + Misses 71129 70834 -295 + Partials 6759 6673 -86 
FlagCoverage Δ
Debug68.51% <39.47%> (-0.08%)⬇️
production62.93% <100.00%> (-0.04%)⬇️
test89.00% <24.59%> (-0.12%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
test/Microsoft.ML.AutoML.Tests/AutoFitTests.cs84.94% <ø> (+2.46%)⬆️
...icrosoft.ML.AutoML.Tests/TrialResultManagerTest.cs0.00% <0.00%> (ø)
src/Microsoft.ML.SearchSpace/SearchSpace.cs73.36% <100.00%> (+1.34%)⬆️
src/Microsoft.ML.SearchSpace/Tuner/RandomTuner.cs60.00% <100.00%> (-21.82%)⬇️
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs96.23% <100.00%> (+3.84%)⬆️
.../Microsoft.ML.AutoML.Tests/CostFrugalTunerTests.cs97.20% <100.00%> (+1.30%)⬆️
.../Microsoft.ML.SearchSpace.Tests/SearchSpaceTest.cs100.00% <100.00%> (ø)
src/Microsoft.ML.Maml/MAML.cs24.36% <0.00%> (-2.54%)⬇️
src/Microsoft.Data.Analysis/DataFrame.IO.cs79.88% <0.00%> (-1.28%)⬇️
... and 19 more
@LittleLittleCloud
LittleLittleCloud
commented
Sep 23, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 23, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 24, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 24, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 28, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 28, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Sep 29, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Sep 29, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 4, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
Update AutoMLExperimentTests.cs
436a05c
disable SweepablePipeline_AutoFit_UCI_Adult_CrossValidation_10_Test
@LittleLittleCloud
LittleLittleCloud
commented
Oct 5, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 5, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 6, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 6, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp --help
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Command '--help' is not supported by Azure Pipelines.

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"


See additional documentation.
@LittleLittleCloud
LittleLittleCloud
commented
Oct 10, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run
@azure-pipelines
azure-pipelinesBot
commented
Oct 10, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 2 pipeline(s).
@LittleLittleCloud
LittleLittleCloud
merged commit cba0162
into
dotnet:mainOct 11, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Nov 11, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

Assignees

@LittleLittleCloudLittleLittleCloud

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@LittleLittleCloud@tarekgh@dakersnar