[tune] Fix performance issue and fix reuse tests #4379
Conversation
|
Test PASSed. |
|
Test FAILed. |
|
Test PASSed. |
| self.config = config | ||
| self.num_resets = 0 | ||
| self.iter = 0 | ||
| def create_resettable_class(): |
There was a problem hiding this comment.
This is needed because pytest does not import classes correctly
| self._resources_initialized = True | ||
|
|
||
| def has_resources(self, resources): | ||
| """Returns whether this runner has at least the specified resources.""" | ||
| self._update_avail_resources() | ||
| if time.time() - self._last_resource_refresh > self._refresh_period: |
There was a problem hiding this comment.
Does this mean that if my trials run really fast (say 10ms) I won't be able to schedule more than 2 a second?
Why not refresh after each event processed?
There was a problem hiding this comment.
Oh, IIUC this is just for the "cluster resources", which are not expected to change dynamically often. If that's right then it seems ok, but I would document this.
python/ray/tune/trial_runner.py
Outdated
| return ("Unknown memory usage. Please run `pip install psutil` " | ||
| global _MEMORY_USAGE_WARNED | ||
| if not _MEMORY_USAGE_WARNED: | ||
| _MEMORY_USAGE_WARNED = True |
There was a problem hiding this comment.
Consider reverting this change. The user can easily miss this warning otherwise since it's relatively unobtrusive.
|
Test PASSed. |
|
Test PASSed. |
|
Test FAILed. |
Pytest I think uses some multi-threaded importing, so
classes often fail to import, hence the hanging on Jenkins. This gets around that
but using a constructor.
Performance issue reported earlier is also mitigated by only updating resources
once per TrialRunner step (Fixes #4366).
This also avoids a recurring warning.