fix: handle engine_cache kwarg as alias for custom_engine_cache and tolerate missing cache fields#4230
fix: handle engine_cache kwarg as alias for custom_engine_cache and tolerate missing cache fields#4230anishesg wants to merge 2 commits intopytorch:mainfrom
engine_cache kwarg as alias for custom_engine_cache and tolerate missing cache fields#4230Conversation
…nd tolerate missing cache fields ## Description Signed-off-by: anish k <ak8686@princeton.edu>
github-actions
Bot
added
component: core
| if "engine_cache" in kwargs.keys(): | ||
| warnings.warn( | ||
| "`engine_cache` is deprecated. Please use `custom_engine_cache` to provide a custom engine cache instance.", | ||
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) | ||
| if custom_engine_cache is not None: | ||
| raise ValueError( | ||
| "Use flag `custom_engine_cache` only. Flag `engine_cache` is deprecated." | ||
| ) | ||
| else: | ||
| custom_engine_cache = kwargs["engine_cache"] | ||
|
|
There was a problem hiding this comment.
Hi @anishesg, Thanks for pointing this out.
I reviewed the codebase and found the root cause of the issue is that this tutorial brought a wrong arg name, i.e., engine_cache. We should never use engine_cache because the arg name is always custom_engine_cache, so there's no need to add the warning in _compiler.py. Can you also correct the tutorial's arg name in this PR? Thanks
There was a problem hiding this comment.
good catch — removed all three engine_cache aliasing blocks from _compiler.py and fixed both occurrences in the tutorial (custom_engine_cache in the DiskEngineCache example and the custom backend example). The _engine_cache.py .get() fix is still there since that's a legit backward-compat issue on its own.
There was a problem hiding this comment.
Yeah, backward-compat looks good. I'll merge after CI pass. Thanks for the contribution!
There was a problem hiding this comment.
Yeah, makes sense. I'll update the tutorial to use custom_engine_cache instead and remove the warning.
…piler.py and fix tutorial to use correct custom_engine_cache arg name Signed-off-by: anish k <ak8686@princeton.edu>
github-actions
Bot
added
the
documentation
2 participants
Description
Two bugs in the engine caching path are fixed here.
Bug 1 – tutorial used wrong kwarg name
engine_cache.The
engine_cache.rsttutorial was passingengine_cache=my_cachetocompile(), but the correct parameter name has always beencustom_engine_cache. This caused users following the tutorial to silently get the defaultDiskEngineCachepointing at the system temp dir instead of their configured cache. Fixed both occurrences in the tutorial (theDiskEngineCacheexample and the custom backend example) to usecustom_engine_cache.Bug 2 – cache load fails for engines serialized before
requires_native_multidevicewas added.BaseEngineCache.unpackaccessedunpacked["requires_native_multidevice"]with a hard key lookup, raising aKeyErrorwhen loading a blob that was written by an older version of the library (prior to #4183) that did not include that field. Changed the lookup tounpacked.get("requires_native_multidevice", False)so existing cached blobs can still be loaded.The root-cause files are
docsrc/tutorials/resource_memory/engine_cache.rst(wrong kwarg name) andpy/torch_tensorrt/dynamo/_engine_cache.py(unpackkey lookup).Type of change
Checklist
Fixes #4226