Add SetMaximumMemoryUsageInMegaByte in AutoMLExperiment

[LittleLittleCloud]

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.

This PR adds SetMaximumMemoryUsageInMegaByte, which sets limit to the maximum memory a trial can use, and cancel that running trial if it exceeds the maximum memory limitation.

#6293

Example

// set maximum memory usage to 8gbexperiment.SetMaximumMemoryUsageInMegaByte(8*1024);

How it work

AutoMLExperiment monitors the memory and cpu usage periodically (the default period is 2 seconds) using IPerformanceMonitor, and once the trial exceed the boundary, AutoMLExperiment cancel that trial via MLContext.CancelExecution and does clean up.

Known limitation

This requires estimator support cancel during training by checking if context is still alive from time to time, which is not true for all estimators in ML.Net.
For example, LGBM can't check if context is alive in unmanaged code part, which makes it can't be cancelled during the time when it's calling into native LightGbm dll. In that situation, tiral memory usage might still exceed maximum limit.

[luisquintanilla]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

[LittleLittleCloud]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

Only current trial will be cancelled

GC will be called every time a trial get completed to clean up and release memory, no matter if that trial is successful or not. So enforcing memory limit won't affect GC kick frequency.

[codecov]

Codecov Report

Merging #6305 (5496403) into main (9652e59) will increase coverage by 0.06%.
The diff coverage is 92.90%.

Additional details and impacted files

@@ Coverage Diff @@## main #6305 +/- ##
==========================================
+ Coverage 68.56% 68.63% +0.06% 
==========================================
Files 1170 1170 Lines 247158 247310 +152 Branches 25675 25683 +8 ==========================================
+ Hits 169475 169732 +257 + Misses 70940 70849 -91 + Partials 6743 6729 -14 

Flag	Coverage Δ
Debug	68.63% <92.90%> (+0.06%)	⬆️
production	63.06% <100.00%> (+0.06%)	⬆️
test	89.05% <92.41%> (-0.02%)	⬇️

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

Impacted Files	Coverage Δ
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs	92.38% <87.64%> (-7.62%)	⬇️
src/Microsoft.ML.FastTree/FastTree.cs	80.48% <100.00%> (+0.07%)	⬆️
src/Microsoft.ML.LightGbm/LightGbmTrainerBase.cs	79.77% <100.00%> (+0.30%)	⬆️
...c/Microsoft.ML.LightGbm/WrappedLightGbmTraining.cs	45.45% <100.00%> (+0.71%)	⬆️
...osoft.ML.Tests/TrainerEstimators/TreeEstimators.cs	97.80% <100.00%> (+0.19%)	⬆️
src/Microsoft.ML.Maml/MAML.cs	24.36% <0.00%> (-2.54%)	⬇️
src/Microsoft.ML.Data/Utils/LossFunctions.cs	67.35% <0.00%> (+0.51%)	⬆️
...oft.ML.StandardTrainers/Standard/SdcaMulticlass.cs	92.49% <0.00%> (+1.02%)	⬆️
... and 4 more

[tarekgh]

BinaryClassificationRunner

Should we use IDisposable with this class?

[LittleLittleCloud]

That's a great question here. The IDisposable pattern is mainly because I want to make sure MLContext.CancelExecuation get called and set to null after the trial is finished while I don't want to explicitly call it's deconstructor or call GC. But I can go another route if you have any recommendation.

[tarekgh]

Dispose pattern is not only for GC finalization or deconstruction. It is used to clean up any object when done using it. You already implemented the Dispose method. Having IDisposable will benefit with the using () code pattern.

[tarekgh]

value

what happen if someone passed NaN value? can you try it and look what you'll get?

[LittleLittleCloud]

I will add a check here to make sure it's either not NaN or non-positive.

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

what happen if 2 threads called this in same time? or this is not supported scenario?

Reply

It's not designed to support on AutoMLExperiment level. So if user wants to launch several AutoMLExperiment on multi-thread, they need to create new AutoMLExperiment to do that.

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

)_settings.MaxExperimentTimeInSeconds * 1000

Any concern with overflowing this calculation?

[LittleLittleCloud]

Thanks for catching that, switch to TimeSpan.FromSeconds to avoid overflow in int

[LittleLittleCloud]

Resolved

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

Also, Did we think making _globalCancellationTokenSource not class field and only having it as local inside RunAsync? The current code cause problems in multi-threading scenarios.

---

In reply to: 1233449717

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

Environment.ProcessorCount * _checkIntervalInMilliseconds

nit: This can be calculated once in the constructor.

[LittleLittleCloud]

compiler should be able to optimize that.

[tarekgh]

Are you assuming that? or you have tried it?

Trying quickly in csharplab I am seeing it is not optimized. note I didn't use Environment.ProcessorCount because it is just 1 and will be optimized at that time. so, I used other constant.

Anyway, it is a minor point, and you can ignore it if you are ok with the current code.

[LittleLittleCloud]

Yeah indeed it's not, turns out I'm too optimistic on the capability of how much optimization compiler can be.

Thanks for introducing csharplab, that's such a wonderful tool

[tarekgh]

why we are doing that?

[LittleLittleCloud]

You mean GC.Collect or GC.AddMemoryPressure part?

[tarekgh]

yes

[LittleLittleCloud]

This is because if we hit that code, it means the current trial uses memory that exceed the limit. Therefore we would like to run a GC to collect memory after that trial get cancelled.

GC.AddMemoryPressure is mostly for LightGbm which might allocate large chunk of unmanaged memory.

[LittleLittleCloud]

Resolved

[tarekgh]

Wouldn't GC kick off automatically when it needs more memory? at that time will collect any stale objects. I am asking why you are forcing that to happen at this time.

[LittleLittleCloud]

But it wouldn't if GC thinks it still has enough memory. And in that case, the next trial will likely to be canceled even if it's not using a lot of memory because the memory used by the last trial is not released.

[tarekgh]

But it wouldn't if GC thinks it still has enough memory.

Could you please talk more about that? Why GC will think there will not be enough memory? I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

I am not objecting here, I am trying to understand the case.

[LittleLittleCloud]

I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

Only if GC thinks there's not enough memory and runs a collection, but that's not always the case when a trial gets canceled. For example, if the user sets a very small number for maximum memory usage.

[tarekgh]

I admit I am not fully understanding the whole picture :-) I'll leave it to you if you are sure about that. You may ignore my comment then.

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

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.

This PR adds SetMaximumMemoryUsageInMegaByte, which sets limit to the maximum memory a trial can use, and cancel that running trial if it exceeds the maximum memory limitation.

#6293

Example

// set maximum memory usage to 8gbexperiment.SetMaximumMemoryUsageInMegaByte(8*1024);

How it work

AutoMLExperiment monitors the memory and cpu usage periodically (the default period is 2 seconds) using IPerformanceMonitor, and once the trial exceed the boundary, AutoMLExperiment cancel that trial via MLContext.CancelExecution and does clean up.

Known limitation

This requires estimator support cancel during training by checking if context is still alive from time to time, which is not true for all estimators in ML.Net.
For example, LGBM can't check if context is alive in unmanaged code part, which makes it can't be cancelled during the time when it's calling into native LightGbm dll. In that situation, tiral memory usage might still exceed maximum limit.

[luisquintanilla]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

[LittleLittleCloud]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

Only current trial will be cancelled

GC will be called every time a trial get completed to clean up and release memory, no matter if that trial is successful or not. So enforcing memory limit won't affect GC kick frequency.

[codecov]

Codecov Report

Merging #6305 (5496403) into main (9652e59) will increase coverage by 0.06%.
The diff coverage is 92.90%.

Additional details and impacted files

@@ Coverage Diff @@## main #6305 +/- ##
==========================================
+ Coverage 68.56% 68.63% +0.06% 
==========================================
Files 1170 1170 Lines 247158 247310 +152 Branches 25675 25683 +8 ==========================================
+ Hits 169475 169732 +257 + Misses 70940 70849 -91 + Partials 6743 6729 -14 

Flag	Coverage Δ
Debug	68.63% <92.90%> (+0.06%)	⬆️
production	63.06% <100.00%> (+0.06%)	⬆️
test	89.05% <92.41%> (-0.02%)	⬇️

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

Impacted Files	Coverage Δ
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs	92.38% <87.64%> (-7.62%)	⬇️
src/Microsoft.ML.FastTree/FastTree.cs	80.48% <100.00%> (+0.07%)	⬆️
src/Microsoft.ML.LightGbm/LightGbmTrainerBase.cs	79.77% <100.00%> (+0.30%)	⬆️
...c/Microsoft.ML.LightGbm/WrappedLightGbmTraining.cs	45.45% <100.00%> (+0.71%)	⬆️
...osoft.ML.Tests/TrainerEstimators/TreeEstimators.cs	97.80% <100.00%> (+0.19%)	⬆️
src/Microsoft.ML.Maml/MAML.cs	24.36% <0.00%> (-2.54%)	⬇️
src/Microsoft.ML.Data/Utils/LossFunctions.cs	67.35% <0.00%> (+0.51%)	⬆️
...oft.ML.StandardTrainers/Standard/SdcaMulticlass.cs	92.49% <0.00%> (+1.02%)	⬆️
... and 4 more

[tarekgh]

BinaryClassificationRunner

Should we use IDisposable with this class?

[LittleLittleCloud]

That's a great question here. The IDisposable pattern is mainly because I want to make sure MLContext.CancelExecuation get called and set to null after the trial is finished while I don't want to explicitly call it's deconstructor or call GC. But I can go another route if you have any recommendation.

[tarekgh]

Dispose pattern is not only for GC finalization or deconstruction. It is used to clean up any object when done using it. You already implemented the Dispose method. Having IDisposable will benefit with the using () code pattern.

[tarekgh]

value

what happen if someone passed NaN value? can you try it and look what you'll get?

[LittleLittleCloud]

I will add a check here to make sure it's either not NaN or non-positive.

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

what happen if 2 threads called this in same time? or this is not supported scenario?

Reply

It's not designed to support on AutoMLExperiment level. So if user wants to launch several AutoMLExperiment on multi-thread, they need to create new AutoMLExperiment to do that.

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

)_settings.MaxExperimentTimeInSeconds * 1000

Any concern with overflowing this calculation?

[LittleLittleCloud]

Thanks for catching that, switch to TimeSpan.FromSeconds to avoid overflow in int

[LittleLittleCloud]

Resolved

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

Also, Did we think making _globalCancellationTokenSource not class field and only having it as local inside RunAsync? The current code cause problems in multi-threading scenarios.

---

In reply to: 1233449717

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

Environment.ProcessorCount * _checkIntervalInMilliseconds

nit: This can be calculated once in the constructor.

[LittleLittleCloud]

compiler should be able to optimize that.

[tarekgh]

Are you assuming that? or you have tried it?

Trying quickly in csharplab I am seeing it is not optimized. note I didn't use Environment.ProcessorCount because it is just 1 and will be optimized at that time. so, I used other constant.

Anyway, it is a minor point, and you can ignore it if you are ok with the current code.

[LittleLittleCloud]

Yeah indeed it's not, turns out I'm too optimistic on the capability of how much optimization compiler can be.

Thanks for introducing csharplab, that's such a wonderful tool

[tarekgh]

why we are doing that?

[LittleLittleCloud]

You mean GC.Collect or GC.AddMemoryPressure part?

[tarekgh]

yes

[LittleLittleCloud]

This is because if we hit that code, it means the current trial uses memory that exceed the limit. Therefore we would like to run a GC to collect memory after that trial get cancelled.

GC.AddMemoryPressure is mostly for LightGbm which might allocate large chunk of unmanaged memory.

[LittleLittleCloud]

Resolved

[tarekgh]

Wouldn't GC kick off automatically when it needs more memory? at that time will collect any stale objects. I am asking why you are forcing that to happen at this time.

[LittleLittleCloud]

But it wouldn't if GC thinks it still has enough memory. And in that case, the next trial will likely to be canceled even if it's not using a lot of memory because the memory used by the last trial is not released.

[tarekgh]

But it wouldn't if GC thinks it still has enough memory.

Could you please talk more about that? Why GC will think there will not be enough memory? I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

I am not objecting here, I am trying to understand the case.

[LittleLittleCloud]

I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

Only if GC thinks there's not enough memory and runs a collection, but that's not always the case when a trial gets canceled. For example, if the user sets a very small number for maximum memory usage.

[tarekgh]

I admit I am not fully understanding the whole picture :-) I'll leave it to you if you are sure about that. You may ignore my comment then.

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

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.

This PR adds SetMaximumMemoryUsageInMegaByte, which sets limit to the maximum memory a trial can use, and cancel that running trial if it exceeds the maximum memory limitation.

#6293

Example

// set maximum memory usage to 8gbexperiment.SetMaximumMemoryUsageInMegaByte(8*1024);

How it work

AutoMLExperiment monitors the memory and cpu usage periodically (the default period is 2 seconds) using IPerformanceMonitor, and once the trial exceed the boundary, AutoMLExperiment cancel that trial via MLContext.CancelExecution and does clean up.

Known limitation

This requires estimator support cancel during training by checking if context is still alive from time to time, which is not true for all estimators in ML.Net.
For example, LGBM can't check if context is alive in unmanaged code part, which makes it can't be cancelled during the time when it's calling into native LightGbm dll. In that situation, tiral memory usage might still exceed maximum limit.

[luisquintanilla]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

[LittleLittleCloud]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

Only current trial will be cancelled

GC will be called every time a trial get completed to clean up and release memory, no matter if that trial is successful or not. So enforcing memory limit won't affect GC kick frequency.

[codecov]

Codecov Report

Merging #6305 (5496403) into main (9652e59) will increase coverage by 0.06%.
The diff coverage is 92.90%.

Additional details and impacted files

@@ Coverage Diff @@## main #6305 +/- ##
==========================================
+ Coverage 68.56% 68.63% +0.06% 
==========================================
Files 1170 1170 Lines 247158 247310 +152 Branches 25675 25683 +8 ==========================================
+ Hits 169475 169732 +257 + Misses 70940 70849 -91 + Partials 6743 6729 -14 

Flag	Coverage Δ
Debug	68.63% <92.90%> (+0.06%)	⬆️
production	63.06% <100.00%> (+0.06%)	⬆️
test	89.05% <92.41%> (-0.02%)	⬇️

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

Impacted Files	Coverage Δ
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs	92.38% <87.64%> (-7.62%)	⬇️
src/Microsoft.ML.FastTree/FastTree.cs	80.48% <100.00%> (+0.07%)	⬆️
src/Microsoft.ML.LightGbm/LightGbmTrainerBase.cs	79.77% <100.00%> (+0.30%)	⬆️
...c/Microsoft.ML.LightGbm/WrappedLightGbmTraining.cs	45.45% <100.00%> (+0.71%)	⬆️
...osoft.ML.Tests/TrainerEstimators/TreeEstimators.cs	97.80% <100.00%> (+0.19%)	⬆️
src/Microsoft.ML.Maml/MAML.cs	24.36% <0.00%> (-2.54%)	⬇️
src/Microsoft.ML.Data/Utils/LossFunctions.cs	67.35% <0.00%> (+0.51%)	⬆️
...oft.ML.StandardTrainers/Standard/SdcaMulticlass.cs	92.49% <0.00%> (+1.02%)	⬆️
... and 4 more

[tarekgh]

BinaryClassificationRunner

Should we use IDisposable with this class?

[LittleLittleCloud]

That's a great question here. The IDisposable pattern is mainly because I want to make sure MLContext.CancelExecuation get called and set to null after the trial is finished while I don't want to explicitly call it's deconstructor or call GC. But I can go another route if you have any recommendation.

[tarekgh]

Dispose pattern is not only for GC finalization or deconstruction. It is used to clean up any object when done using it. You already implemented the Dispose method. Having IDisposable will benefit with the using () code pattern.

[tarekgh]

value

what happen if someone passed NaN value? can you try it and look what you'll get?

[LittleLittleCloud]

I will add a check here to make sure it's either not NaN or non-positive.

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

what happen if 2 threads called this in same time? or this is not supported scenario?

Reply

It's not designed to support on AutoMLExperiment level. So if user wants to launch several AutoMLExperiment on multi-thread, they need to create new AutoMLExperiment to do that.

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

)_settings.MaxExperimentTimeInSeconds * 1000

Any concern with overflowing this calculation?

[LittleLittleCloud]

Thanks for catching that, switch to TimeSpan.FromSeconds to avoid overflow in int

[LittleLittleCloud]

Resolved

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

Also, Did we think making _globalCancellationTokenSource not class field and only having it as local inside RunAsync? The current code cause problems in multi-threading scenarios.

---

In reply to: 1233449717

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

Environment.ProcessorCount * _checkIntervalInMilliseconds

nit: This can be calculated once in the constructor.

[LittleLittleCloud]

compiler should be able to optimize that.

[tarekgh]

Are you assuming that? or you have tried it?

Trying quickly in csharplab I am seeing it is not optimized. note I didn't use Environment.ProcessorCount because it is just 1 and will be optimized at that time. so, I used other constant.

Anyway, it is a minor point, and you can ignore it if you are ok with the current code.

[LittleLittleCloud]

Yeah indeed it's not, turns out I'm too optimistic on the capability of how much optimization compiler can be.

Thanks for introducing csharplab, that's such a wonderful tool

[tarekgh]

why we are doing that?

[LittleLittleCloud]

You mean GC.Collect or GC.AddMemoryPressure part?

[tarekgh]

yes

[LittleLittleCloud]

This is because if we hit that code, it means the current trial uses memory that exceed the limit. Therefore we would like to run a GC to collect memory after that trial get cancelled.

GC.AddMemoryPressure is mostly for LightGbm which might allocate large chunk of unmanaged memory.

[LittleLittleCloud]

Resolved

[tarekgh]

Wouldn't GC kick off automatically when it needs more memory? at that time will collect any stale objects. I am asking why you are forcing that to happen at this time.

[LittleLittleCloud]

But it wouldn't if GC thinks it still has enough memory. And in that case, the next trial will likely to be canceled even if it's not using a lot of memory because the memory used by the last trial is not released.

[tarekgh]

But it wouldn't if GC thinks it still has enough memory.

Could you please talk more about that? Why GC will think there will not be enough memory? I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

I am not objecting here, I am trying to understand the case.

[LittleLittleCloud]

I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

Only if GC thinks there's not enough memory and runs a collection, but that's not always the case when a trial gets canceled. For example, if the user sets a very small number for maximum memory usage.

[tarekgh]

I admit I am not fully understanding the whole picture :-) I'll leave it to you if you are sure about that. You may ignore my comment then.

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

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.

This PR adds SetMaximumMemoryUsageInMegaByte, which sets limit to the maximum memory a trial can use, and cancel that running trial if it exceeds the maximum memory limitation.

#6293

Example

// set maximum memory usage to 8gbexperiment.SetMaximumMemoryUsageInMegaByte(8*1024);

How it work

AutoMLExperiment monitors the memory and cpu usage periodically (the default period is 2 seconds) using IPerformanceMonitor, and once the trial exceed the boundary, AutoMLExperiment cancel that trial via MLContext.CancelExecution and does clean up.

Known limitation

This requires estimator support cancel during training by checking if context is still alive from time to time, which is not true for all estimators in ML.Net.
For example, LGBM can't check if context is alive in unmanaged code part, which makes it can't be cancelled during the time when it's calling into native LightGbm dll. In that situation, tiral memory usage might still exceed maximum limit.

[luisquintanilla]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

[LittleLittleCloud]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

Only current trial will be cancelled

GC will be called every time a trial get completed to clean up and release memory, no matter if that trial is successful or not. So enforcing memory limit won't affect GC kick frequency.

[codecov]

Codecov Report

Merging #6305 (5496403) into main (9652e59) will increase coverage by 0.06%.
The diff coverage is 92.90%.

Additional details and impacted files

@@ Coverage Diff @@## main #6305 +/- ##
==========================================
+ Coverage 68.56% 68.63% +0.06% 
==========================================
Files 1170 1170 Lines 247158 247310 +152 Branches 25675 25683 +8 ==========================================
+ Hits 169475 169732 +257 + Misses 70940 70849 -91 + Partials 6743 6729 -14 

Flag	Coverage Δ
Debug	68.63% <92.90%> (+0.06%)	⬆️
production	63.06% <100.00%> (+0.06%)	⬆️
test	89.05% <92.41%> (-0.02%)	⬇️

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

Impacted Files	Coverage Δ
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs	92.38% <87.64%> (-7.62%)	⬇️
src/Microsoft.ML.FastTree/FastTree.cs	80.48% <100.00%> (+0.07%)	⬆️
src/Microsoft.ML.LightGbm/LightGbmTrainerBase.cs	79.77% <100.00%> (+0.30%)	⬆️
...c/Microsoft.ML.LightGbm/WrappedLightGbmTraining.cs	45.45% <100.00%> (+0.71%)	⬆️
...osoft.ML.Tests/TrainerEstimators/TreeEstimators.cs	97.80% <100.00%> (+0.19%)	⬆️
src/Microsoft.ML.Maml/MAML.cs	24.36% <0.00%> (-2.54%)	⬇️
src/Microsoft.ML.Data/Utils/LossFunctions.cs	67.35% <0.00%> (+0.51%)	⬆️
...oft.ML.StandardTrainers/Standard/SdcaMulticlass.cs	92.49% <0.00%> (+1.02%)	⬆️
... and 4 more

[tarekgh]

BinaryClassificationRunner

Should we use IDisposable with this class?

[LittleLittleCloud]

That's a great question here. The IDisposable pattern is mainly because I want to make sure MLContext.CancelExecuation get called and set to null after the trial is finished while I don't want to explicitly call it's deconstructor or call GC. But I can go another route if you have any recommendation.

[tarekgh]

Dispose pattern is not only for GC finalization or deconstruction. It is used to clean up any object when done using it. You already implemented the Dispose method. Having IDisposable will benefit with the using () code pattern.

[tarekgh]

value

what happen if someone passed NaN value? can you try it and look what you'll get?

[LittleLittleCloud]

I will add a check here to make sure it's either not NaN or non-positive.

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

what happen if 2 threads called this in same time? or this is not supported scenario?

Reply

It's not designed to support on AutoMLExperiment level. So if user wants to launch several AutoMLExperiment on multi-thread, they need to create new AutoMLExperiment to do that.

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

)_settings.MaxExperimentTimeInSeconds * 1000

Any concern with overflowing this calculation?

[LittleLittleCloud]

Thanks for catching that, switch to TimeSpan.FromSeconds to avoid overflow in int

[LittleLittleCloud]

Resolved

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

Also, Did we think making _globalCancellationTokenSource not class field and only having it as local inside RunAsync? The current code cause problems in multi-threading scenarios.

---

In reply to: 1233449717

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

Environment.ProcessorCount * _checkIntervalInMilliseconds

nit: This can be calculated once in the constructor.

[LittleLittleCloud]

compiler should be able to optimize that.

[tarekgh]

Are you assuming that? or you have tried it?

Trying quickly in csharplab I am seeing it is not optimized. note I didn't use Environment.ProcessorCount because it is just 1 and will be optimized at that time. so, I used other constant.

Anyway, it is a minor point, and you can ignore it if you are ok with the current code.

[LittleLittleCloud]

Yeah indeed it's not, turns out I'm too optimistic on the capability of how much optimization compiler can be.

Thanks for introducing csharplab, that's such a wonderful tool

[tarekgh]

why we are doing that?

[LittleLittleCloud]

You mean GC.Collect or GC.AddMemoryPressure part?

[tarekgh]

yes

[LittleLittleCloud]

This is because if we hit that code, it means the current trial uses memory that exceed the limit. Therefore we would like to run a GC to collect memory after that trial get cancelled.

GC.AddMemoryPressure is mostly for LightGbm which might allocate large chunk of unmanaged memory.

[LittleLittleCloud]

Resolved

[tarekgh]

Wouldn't GC kick off automatically when it needs more memory? at that time will collect any stale objects. I am asking why you are forcing that to happen at this time.

[LittleLittleCloud]

But it wouldn't if GC thinks it still has enough memory. And in that case, the next trial will likely to be canceled even if it's not using a lot of memory because the memory used by the last trial is not released.

[tarekgh]

But it wouldn't if GC thinks it still has enough memory.

Could you please talk more about that? Why GC will think there will not be enough memory? I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

I am not objecting here, I am trying to understand the case.

[LittleLittleCloud]

I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

Only if GC thinks there's not enough memory and runs a collection, but that's not always the case when a trial gets canceled. For example, if the user sets a very small number for maximum memory usage.

[tarekgh]

I admit I am not fully understanding the whole picture :-) I'll leave it to you if you are sure about that. You may ignore my comment then.

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

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.

This PR adds SetMaximumMemoryUsageInMegaByte, which sets limit to the maximum memory a trial can use, and cancel that running trial if it exceeds the maximum memory limitation.

#6293

Example

// set maximum memory usage to 8gbexperiment.SetMaximumMemoryUsageInMegaByte(8*1024);

How it work

AutoMLExperiment monitors the memory and cpu usage periodically (the default period is 2 seconds) using IPerformanceMonitor, and once the trial exceed the boundary, AutoMLExperiment cancel that trial via MLContext.CancelExecution and does clean up.

Known limitation

This requires estimator support cancel during training by checking if context is still alive from time to time, which is not true for all estimators in ML.Net.
For example, LGBM can't check if context is alive in unmanaged code part, which makes it can't be cancelled during the time when it's calling into native LightGbm dll. In that situation, tiral memory usage might still exceed maximum limit.

[luisquintanilla]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

[LittleLittleCloud]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

Only current trial will be cancelled

GC will be called every time a trial get completed to clean up and release memory, no matter if that trial is successful or not. So enforcing memory limit won't affect GC kick frequency.

[codecov]

Codecov Report

Merging #6305 (5496403) into main (9652e59) will increase coverage by 0.06%.
The diff coverage is 92.90%.

Additional details and impacted files

@@ Coverage Diff @@## main #6305 +/- ##
==========================================
+ Coverage 68.56% 68.63% +0.06% 
==========================================
Files 1170 1170 Lines 247158 247310 +152 Branches 25675 25683 +8 ==========================================
+ Hits 169475 169732 +257 + Misses 70940 70849 -91 + Partials 6743 6729 -14 

Flag	Coverage Δ
Debug	68.63% <92.90%> (+0.06%)	⬆️
production	63.06% <100.00%> (+0.06%)	⬆️
test	89.05% <92.41%> (-0.02%)	⬇️

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

Impacted Files	Coverage Δ
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs	92.38% <87.64%> (-7.62%)	⬇️
src/Microsoft.ML.FastTree/FastTree.cs	80.48% <100.00%> (+0.07%)	⬆️
src/Microsoft.ML.LightGbm/LightGbmTrainerBase.cs	79.77% <100.00%> (+0.30%)	⬆️
...c/Microsoft.ML.LightGbm/WrappedLightGbmTraining.cs	45.45% <100.00%> (+0.71%)	⬆️
...osoft.ML.Tests/TrainerEstimators/TreeEstimators.cs	97.80% <100.00%> (+0.19%)	⬆️
src/Microsoft.ML.Maml/MAML.cs	24.36% <0.00%> (-2.54%)	⬇️
src/Microsoft.ML.Data/Utils/LossFunctions.cs	67.35% <0.00%> (+0.51%)	⬆️
...oft.ML.StandardTrainers/Standard/SdcaMulticlass.cs	92.49% <0.00%> (+1.02%)	⬆️
... and 4 more

[tarekgh]

BinaryClassificationRunner

Should we use IDisposable with this class?

[LittleLittleCloud]

That's a great question here. The IDisposable pattern is mainly because I want to make sure MLContext.CancelExecuation get called and set to null after the trial is finished while I don't want to explicitly call it's deconstructor or call GC. But I can go another route if you have any recommendation.

[tarekgh]

Dispose pattern is not only for GC finalization or deconstruction. It is used to clean up any object when done using it. You already implemented the Dispose method. Having IDisposable will benefit with the using () code pattern.

[tarekgh]

value

what happen if someone passed NaN value? can you try it and look what you'll get?

[LittleLittleCloud]

I will add a check here to make sure it's either not NaN or non-positive.

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

what happen if 2 threads called this in same time? or this is not supported scenario?

Reply

It's not designed to support on AutoMLExperiment level. So if user wants to launch several AutoMLExperiment on multi-thread, they need to create new AutoMLExperiment to do that.

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

)_settings.MaxExperimentTimeInSeconds * 1000

Any concern with overflowing this calculation?

[LittleLittleCloud]

Thanks for catching that, switch to TimeSpan.FromSeconds to avoid overflow in int

[LittleLittleCloud]

Resolved

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

Also, Did we think making _globalCancellationTokenSource not class field and only having it as local inside RunAsync? The current code cause problems in multi-threading scenarios.

---

In reply to: 1233449717

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

Environment.ProcessorCount * _checkIntervalInMilliseconds

nit: This can be calculated once in the constructor.

[LittleLittleCloud]

compiler should be able to optimize that.

[tarekgh]

Are you assuming that? or you have tried it?

Trying quickly in csharplab I am seeing it is not optimized. note I didn't use Environment.ProcessorCount because it is just 1 and will be optimized at that time. so, I used other constant.

Anyway, it is a minor point, and you can ignore it if you are ok with the current code.

[LittleLittleCloud]

Yeah indeed it's not, turns out I'm too optimistic on the capability of how much optimization compiler can be.

Thanks for introducing csharplab, that's such a wonderful tool

[tarekgh]

why we are doing that?

[LittleLittleCloud]

You mean GC.Collect or GC.AddMemoryPressure part?

[tarekgh]

yes

[LittleLittleCloud]

This is because if we hit that code, it means the current trial uses memory that exceed the limit. Therefore we would like to run a GC to collect memory after that trial get cancelled.

GC.AddMemoryPressure is mostly for LightGbm which might allocate large chunk of unmanaged memory.

[LittleLittleCloud]

Resolved

[tarekgh]

Wouldn't GC kick off automatically when it needs more memory? at that time will collect any stale objects. I am asking why you are forcing that to happen at this time.

[LittleLittleCloud]

But it wouldn't if GC thinks it still has enough memory. And in that case, the next trial will likely to be canceled even if it's not using a lot of memory because the memory used by the last trial is not released.

[tarekgh]

But it wouldn't if GC thinks it still has enough memory.

Could you please talk more about that? Why GC will think there will not be enough memory? I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

I am not objecting here, I am trying to understand the case.

[LittleLittleCloud]

I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

Only if GC thinks there's not enough memory and runs a collection, but that's not always the case when a trial gets canceled. For example, if the user sets a very small number for maximum memory usage.

[tarekgh]

I admit I am not fully understanding the whole picture :-) I'll leave it to you if you are sure about that. You may ignore my comment then.

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

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.

This PR adds SetMaximumMemoryUsageInMegaByte, which sets limit to the maximum memory a trial can use, and cancel that running trial if it exceeds the maximum memory limitation.

#6293

Example

// set maximum memory usage to 8gbexperiment.SetMaximumMemoryUsageInMegaByte(8*1024);

How it work

AutoMLExperiment monitors the memory and cpu usage periodically (the default period is 2 seconds) using IPerformanceMonitor, and once the trial exceed the boundary, AutoMLExperiment cancel that trial via MLContext.CancelExecution and does clean up.

Known limitation

This requires estimator support cancel during training by checking if context is still alive from time to time, which is not true for all estimators in ML.Net.
For example, LGBM can't check if context is alive in unmanaged code part, which makes it can't be cancelled during the time when it's calling into native LightGbm dll. In that situation, tiral memory usage might still exceed maximum limit.

[luisquintanilla]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

[LittleLittleCloud]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

Only current trial will be cancelled

GC will be called every time a trial get completed to clean up and release memory, no matter if that trial is successful or not. So enforcing memory limit won't affect GC kick frequency.

[codecov]

Codecov Report

Merging #6305 (5496403) into main (9652e59) will increase coverage by 0.06%.
The diff coverage is 92.90%.

Additional details and impacted files

@@ Coverage Diff @@## main #6305 +/- ##
==========================================
+ Coverage 68.56% 68.63% +0.06% 
==========================================
Files 1170 1170 Lines 247158 247310 +152 Branches 25675 25683 +8 ==========================================
+ Hits 169475 169732 +257 + Misses 70940 70849 -91 + Partials 6743 6729 -14 

Flag	Coverage Δ
Debug	68.63% <92.90%> (+0.06%)	⬆️
production	63.06% <100.00%> (+0.06%)	⬆️
test	89.05% <92.41%> (-0.02%)	⬇️

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

Impacted Files	Coverage Δ
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs	92.38% <87.64%> (-7.62%)	⬇️
src/Microsoft.ML.FastTree/FastTree.cs	80.48% <100.00%> (+0.07%)	⬆️
src/Microsoft.ML.LightGbm/LightGbmTrainerBase.cs	79.77% <100.00%> (+0.30%)	⬆️
...c/Microsoft.ML.LightGbm/WrappedLightGbmTraining.cs	45.45% <100.00%> (+0.71%)	⬆️
...osoft.ML.Tests/TrainerEstimators/TreeEstimators.cs	97.80% <100.00%> (+0.19%)	⬆️
src/Microsoft.ML.Maml/MAML.cs	24.36% <0.00%> (-2.54%)	⬇️
src/Microsoft.ML.Data/Utils/LossFunctions.cs	67.35% <0.00%> (+0.51%)	⬆️
...oft.ML.StandardTrainers/Standard/SdcaMulticlass.cs	92.49% <0.00%> (+1.02%)	⬆️
... and 4 more

[tarekgh]

BinaryClassificationRunner

Should we use IDisposable with this class?

[LittleLittleCloud]

That's a great question here. The IDisposable pattern is mainly because I want to make sure MLContext.CancelExecuation get called and set to null after the trial is finished while I don't want to explicitly call it's deconstructor or call GC. But I can go another route if you have any recommendation.

[tarekgh]

Dispose pattern is not only for GC finalization or deconstruction. It is used to clean up any object when done using it. You already implemented the Dispose method. Having IDisposable will benefit with the using () code pattern.

[tarekgh]

value

what happen if someone passed NaN value? can you try it and look what you'll get?

[LittleLittleCloud]

I will add a check here to make sure it's either not NaN or non-positive.

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

what happen if 2 threads called this in same time? or this is not supported scenario?

Reply

It's not designed to support on AutoMLExperiment level. So if user wants to launch several AutoMLExperiment on multi-thread, they need to create new AutoMLExperiment to do that.

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

)_settings.MaxExperimentTimeInSeconds * 1000

Any concern with overflowing this calculation?

[LittleLittleCloud]

Thanks for catching that, switch to TimeSpan.FromSeconds to avoid overflow in int

[LittleLittleCloud]

Resolved

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

Also, Did we think making _globalCancellationTokenSource not class field and only having it as local inside RunAsync? The current code cause problems in multi-threading scenarios.

---

In reply to: 1233449717

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

Environment.ProcessorCount * _checkIntervalInMilliseconds

nit: This can be calculated once in the constructor.

[LittleLittleCloud]

compiler should be able to optimize that.

[tarekgh]

Are you assuming that? or you have tried it?

Trying quickly in csharplab I am seeing it is not optimized. note I didn't use Environment.ProcessorCount because it is just 1 and will be optimized at that time. so, I used other constant.

Anyway, it is a minor point, and you can ignore it if you are ok with the current code.

[LittleLittleCloud]

Yeah indeed it's not, turns out I'm too optimistic on the capability of how much optimization compiler can be.

Thanks for introducing csharplab, that's such a wonderful tool

[tarekgh]

why we are doing that?

[LittleLittleCloud]

You mean GC.Collect or GC.AddMemoryPressure part?

[tarekgh]

yes

[LittleLittleCloud]

This is because if we hit that code, it means the current trial uses memory that exceed the limit. Therefore we would like to run a GC to collect memory after that trial get cancelled.

GC.AddMemoryPressure is mostly for LightGbm which might allocate large chunk of unmanaged memory.

[LittleLittleCloud]

Resolved

[tarekgh]

Wouldn't GC kick off automatically when it needs more memory? at that time will collect any stale objects. I am asking why you are forcing that to happen at this time.

[LittleLittleCloud]

But it wouldn't if GC thinks it still has enough memory. And in that case, the next trial will likely to be canceled even if it's not using a lot of memory because the memory used by the last trial is not released.

[tarekgh]

But it wouldn't if GC thinks it still has enough memory.

Could you please talk more about that? Why GC will think there will not be enough memory? I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

I am not objecting here, I am trying to understand the case.

[LittleLittleCloud]

I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

Only if GC thinks there's not enough memory and runs a collection, but that's not always the case when a trial gets canceled. For example, if the user sets a very small number for maximum memory usage.

[tarekgh]

I admit I am not fully understanding the whole picture :-) I'll leave it to you if you are sure about that. You may ignore my comment then.

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

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.

This PR adds SetMaximumMemoryUsageInMegaByte, which sets limit to the maximum memory a trial can use, and cancel that running trial if it exceeds the maximum memory limitation.

#6293

Example

// set maximum memory usage to 8gbexperiment.SetMaximumMemoryUsageInMegaByte(8*1024);

How it work

AutoMLExperiment monitors the memory and cpu usage periodically (the default period is 2 seconds) using IPerformanceMonitor, and once the trial exceed the boundary, AutoMLExperiment cancel that trial via MLContext.CancelExecution and does clean up.

Known limitation

This requires estimator support cancel during training by checking if context is still alive from time to time, which is not true for all estimators in ML.Net.
For example, LGBM can't check if context is alive in unmanaged code part, which makes it can't be cancelled during the time when it's calling into native LightGbm dll. In that situation, tiral memory usage might still exceed maximum limit.

[luisquintanilla]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

[LittleLittleCloud]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

Only current trial will be cancelled

GC will be called every time a trial get completed to clean up and release memory, no matter if that trial is successful or not. So enforcing memory limit won't affect GC kick frequency.

[codecov]

Codecov Report

Merging #6305 (5496403) into main (9652e59) will increase coverage by 0.06%.
The diff coverage is 92.90%.

Additional details and impacted files

@@ Coverage Diff @@## main #6305 +/- ##
==========================================
+ Coverage 68.56% 68.63% +0.06% 
==========================================
Files 1170 1170 Lines 247158 247310 +152 Branches 25675 25683 +8 ==========================================
+ Hits 169475 169732 +257 + Misses 70940 70849 -91 + Partials 6743 6729 -14 

Flag	Coverage Δ
Debug	68.63% <92.90%> (+0.06%)	⬆️
production	63.06% <100.00%> (+0.06%)	⬆️
test	89.05% <92.41%> (-0.02%)	⬇️

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

Impacted Files	Coverage Δ
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs	92.38% <87.64%> (-7.62%)	⬇️
src/Microsoft.ML.FastTree/FastTree.cs	80.48% <100.00%> (+0.07%)	⬆️
src/Microsoft.ML.LightGbm/LightGbmTrainerBase.cs	79.77% <100.00%> (+0.30%)	⬆️
...c/Microsoft.ML.LightGbm/WrappedLightGbmTraining.cs	45.45% <100.00%> (+0.71%)	⬆️
...osoft.ML.Tests/TrainerEstimators/TreeEstimators.cs	97.80% <100.00%> (+0.19%)	⬆️
src/Microsoft.ML.Maml/MAML.cs	24.36% <0.00%> (-2.54%)	⬇️
src/Microsoft.ML.Data/Utils/LossFunctions.cs	67.35% <0.00%> (+0.51%)	⬆️
...oft.ML.StandardTrainers/Standard/SdcaMulticlass.cs	92.49% <0.00%> (+1.02%)	⬆️
... and 4 more

[tarekgh]

BinaryClassificationRunner

Should we use IDisposable with this class?

[LittleLittleCloud]

That's a great question here. The IDisposable pattern is mainly because I want to make sure MLContext.CancelExecuation get called and set to null after the trial is finished while I don't want to explicitly call it's deconstructor or call GC. But I can go another route if you have any recommendation.

[tarekgh]

Dispose pattern is not only for GC finalization or deconstruction. It is used to clean up any object when done using it. You already implemented the Dispose method. Having IDisposable will benefit with the using () code pattern.

[tarekgh]

value

what happen if someone passed NaN value? can you try it and look what you'll get?

[LittleLittleCloud]

I will add a check here to make sure it's either not NaN or non-positive.

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

what happen if 2 threads called this in same time? or this is not supported scenario?

Reply

It's not designed to support on AutoMLExperiment level. So if user wants to launch several AutoMLExperiment on multi-thread, they need to create new AutoMLExperiment to do that.

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

)_settings.MaxExperimentTimeInSeconds * 1000

Any concern with overflowing this calculation?

[LittleLittleCloud]

Thanks for catching that, switch to TimeSpan.FromSeconds to avoid overflow in int

[LittleLittleCloud]

Resolved

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

Also, Did we think making _globalCancellationTokenSource not class field and only having it as local inside RunAsync? The current code cause problems in multi-threading scenarios.

---

In reply to: 1233449717

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

Environment.ProcessorCount * _checkIntervalInMilliseconds

nit: This can be calculated once in the constructor.

[LittleLittleCloud]

compiler should be able to optimize that.

[tarekgh]

Are you assuming that? or you have tried it?

Trying quickly in csharplab I am seeing it is not optimized. note I didn't use Environment.ProcessorCount because it is just 1 and will be optimized at that time. so, I used other constant.

Anyway, it is a minor point, and you can ignore it if you are ok with the current code.

[LittleLittleCloud]

Yeah indeed it's not, turns out I'm too optimistic on the capability of how much optimization compiler can be.

Thanks for introducing csharplab, that's such a wonderful tool

[tarekgh]

why we are doing that?

[LittleLittleCloud]

You mean GC.Collect or GC.AddMemoryPressure part?

[tarekgh]

yes

[LittleLittleCloud]

This is because if we hit that code, it means the current trial uses memory that exceed the limit. Therefore we would like to run a GC to collect memory after that trial get cancelled.

GC.AddMemoryPressure is mostly for LightGbm which might allocate large chunk of unmanaged memory.

[LittleLittleCloud]

Resolved

[tarekgh]

Wouldn't GC kick off automatically when it needs more memory? at that time will collect any stale objects. I am asking why you are forcing that to happen at this time.

[LittleLittleCloud]

But it wouldn't if GC thinks it still has enough memory. And in that case, the next trial will likely to be canceled even if it's not using a lot of memory because the memory used by the last trial is not released.

[tarekgh]

But it wouldn't if GC thinks it still has enough memory.

Could you please talk more about that? Why GC will think there will not be enough memory? I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

I am not objecting here, I am trying to understand the case.

[LittleLittleCloud]

I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

Only if GC thinks there's not enough memory and runs a collection, but that's not always the case when a trial gets canceled. For example, if the user sets a very small number for maximum memory usage.

[tarekgh]

I admit I am not fully understanding the whole picture :-) I'll leave it to you if you are sure about that. You may ignore my comment then.

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

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.

This PR adds SetMaximumMemoryUsageInMegaByte, which sets limit to the maximum memory a trial can use, and cancel that running trial if it exceeds the maximum memory limitation.

#6293

Example

// set maximum memory usage to 8gbexperiment.SetMaximumMemoryUsageInMegaByte(8*1024);

How it work

AutoMLExperiment monitors the memory and cpu usage periodically (the default period is 2 seconds) using IPerformanceMonitor, and once the trial exceed the boundary, AutoMLExperiment cancel that trial via MLContext.CancelExecution and does clean up.

Known limitation

This requires estimator support cancel during training by checking if context is still alive from time to time, which is not true for all estimators in ML.Net.
For example, LGBM can't check if context is alive in unmanaged code part, which makes it can't be cancelled during the time when it's calling into native LightGbm dll. In that situation, tiral memory usage might still exceed maximum limit.

[luisquintanilla]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

[LittleLittleCloud]

@LittleLittleCloud looks good to me.

What happens when memory usage hits the specified limit? Is the entire experiment cancelled or only that currently running trial? Does enforcing the memory limit mean that GC kicks in more frequently?

Only current trial will be cancelled

GC will be called every time a trial get completed to clean up and release memory, no matter if that trial is successful or not. So enforcing memory limit won't affect GC kick frequency.

[codecov]

Codecov Report

Merging #6305 (5496403) into main (9652e59) will increase coverage by 0.06%.
The diff coverage is 92.90%.

Additional details and impacted files

@@ Coverage Diff @@## main #6305 +/- ##
==========================================
+ Coverage 68.56% 68.63% +0.06% 
==========================================
Files 1170 1170 Lines 247158 247310 +152 Branches 25675 25683 +8 ==========================================
+ Hits 169475 169732 +257 + Misses 70940 70849 -91 + Partials 6743 6729 -14 

Flag	Coverage Δ
Debug	68.63% <92.90%> (+0.06%)	⬆️
production	63.06% <100.00%> (+0.06%)	⬆️
test	89.05% <92.41%> (-0.02%)	⬇️

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

Impacted Files	Coverage Δ
...Microsoft.ML.AutoML.Tests/AutoMLExperimentTests.cs	92.38% <87.64%> (-7.62%)	⬇️
src/Microsoft.ML.FastTree/FastTree.cs	80.48% <100.00%> (+0.07%)	⬆️
src/Microsoft.ML.LightGbm/LightGbmTrainerBase.cs	79.77% <100.00%> (+0.30%)	⬆️
...c/Microsoft.ML.LightGbm/WrappedLightGbmTraining.cs	45.45% <100.00%> (+0.71%)	⬆️
...osoft.ML.Tests/TrainerEstimators/TreeEstimators.cs	97.80% <100.00%> (+0.19%)	⬆️
src/Microsoft.ML.Maml/MAML.cs	24.36% <0.00%> (-2.54%)	⬇️
src/Microsoft.ML.Data/Utils/LossFunctions.cs	67.35% <0.00%> (+0.51%)	⬆️
...oft.ML.StandardTrainers/Standard/SdcaMulticlass.cs	92.49% <0.00%> (+1.02%)	⬆️
... and 4 more

[tarekgh]

BinaryClassificationRunner

Should we use IDisposable with this class?

[LittleLittleCloud]

That's a great question here. The IDisposable pattern is mainly because I want to make sure MLContext.CancelExecuation get called and set to null after the trial is finished while I don't want to explicitly call it's deconstructor or call GC. But I can go another route if you have any recommendation.

[tarekgh]

Dispose pattern is not only for GC finalization or deconstruction. It is used to clean up any object when done using it. You already implemented the Dispose method. Having IDisposable will benefit with the using () code pattern.

[tarekgh]

value

what happen if someone passed NaN value? can you try it and look what you'll get?

[LittleLittleCloud]

I will add a check here to make sure it's either not NaN or non-positive.

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

what happen if 2 threads called this in same time? or this is not supported scenario?

Reply

It's not designed to support on AutoMLExperiment level. So if user wants to launch several AutoMLExperiment on multi-thread, they need to create new AutoMLExperiment to do that.

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

)_settings.MaxExperimentTimeInSeconds * 1000

Any concern with overflowing this calculation?

[LittleLittleCloud]

Thanks for catching that, switch to TimeSpan.FromSeconds to avoid overflow in int

[LittleLittleCloud]

Resolved

[tarekgh]

 public async Task<TrialResult> RunAsync(CancellationToken ct = default)

Also, Did we think making _globalCancellationTokenSource not class field and only having it as local inside RunAsync? The current code cause problems in multi-threading scenarios.

---

In reply to: 1233449717

---

Refers to: src/Microsoft.ML.AutoML/AutoMLExperiment/AutoMLExperiment.cs:234 in 16eeeae. [](commit_id = 16eeeae, deletion_comment = False)

[tarekgh]

Environment.ProcessorCount * _checkIntervalInMilliseconds

nit: This can be calculated once in the constructor.

[LittleLittleCloud]

compiler should be able to optimize that.

[tarekgh]

Are you assuming that? or you have tried it?

Trying quickly in csharplab I am seeing it is not optimized. note I didn't use Environment.ProcessorCount because it is just 1 and will be optimized at that time. so, I used other constant.

Anyway, it is a minor point, and you can ignore it if you are ok with the current code.

[LittleLittleCloud]

Yeah indeed it's not, turns out I'm too optimistic on the capability of how much optimization compiler can be.

Thanks for introducing csharplab, that's such a wonderful tool

[tarekgh]

why we are doing that?

[LittleLittleCloud]

You mean GC.Collect or GC.AddMemoryPressure part?

[tarekgh]

yes

[LittleLittleCloud]

This is because if we hit that code, it means the current trial uses memory that exceed the limit. Therefore we would like to run a GC to collect memory after that trial get cancelled.

GC.AddMemoryPressure is mostly for LightGbm which might allocate large chunk of unmanaged memory.

[LittleLittleCloud]

Resolved

[tarekgh]

Wouldn't GC kick off automatically when it needs more memory? at that time will collect any stale objects. I am asking why you are forcing that to happen at this time.

[LittleLittleCloud]

But it wouldn't if GC thinks it still has enough memory. And in that case, the next trial will likely to be canceled even if it's not using a lot of memory because the memory used by the last trial is not released.

[tarekgh]

But it wouldn't if GC thinks it still has enough memory.

Could you please talk more about that? Why GC will think there will not be enough memory? I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

I am not objecting here, I am trying to understand the case.

[LittleLittleCloud]

I expect GC will run to collect any unneeded objects at that time which should free a lot of memory. no?

Only if GC thinks there's not enough memory and runs a collection, but that's not always the case when a trial gets canceled. For example, if the user sets a very small number for maximum memory usage.

[tarekgh]

I admit I am not fully understanding the whole picture :-) I'll leave it to you if you are sure about that. You may ignore my comment then.

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[LittleLittleCloud]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).