New features in cloudpickle give unexpected results

[suquark]

I am a developer of Ray, and we have something we want to fix soon. We are updating cloudpickle to 0.6.1 to fix an issue. However, although version 0.6.1 fixed our issue, it failed to pickle several objects correctly. These failures only happened when we were trying to run a complex set of unittests so we found it hard to give some simple instructions to trigger those cases.

I investigated into those cases and created a PR which fixed those failures and this PR showed that:

• When restoring the global variables, in Ray we should always give serialized global variables higher priority (that is, always overriding existing global variables). The new changes in cloudpickle gives higher priority to existing global variables.
• In Ray, do not try to infer global variables from the __module__ attribute of a function. The new changes in cloudpickle try to make use of __module__ when it fails to fetch global variables.

It seems that these new features are not compatible with Ray, but they are not really bugs to be reverted. And since we would like to follow the cloudpickle upstream instead of maintaining our own copy, the proper way should be creating several switches to control the availability of those new features in the upstream. Could you have any suggestions about how and where should we put those switches?

[suquark]

@ogrisel @rgbkrk Could anyone look into this issue? I feel sorry but we want to fix it soon.

[robertnishihara]

@suquark are there things that we can change on the Ray side to fix the problem? Also, do you know what the motivating issues for changing cloudpickle's behavior in these instances were?

[mitar]

@robertnishihara One option could be that Ray would allow one to configure its own version of serialization logic. For example, for us this would then allow us to use plain pickle and side-step all these issues.

[rgbkrk]

I'm not able to look into the issue, nor do I have the necessary context for me to investigate.

[ogrisel]

@suquark I agree we could have switches for customizing this cloudpickle specific behavior.

I think such switches could be passed as arguments to the constructor of the CloudPickler class.

For instance let's add a kwarg named: override_existing_globals=False.

In Ray, do not try to infer global variables from the module attribute of a function. The new changes in cloudpickle try to make use of module when it fails to fetch global variables.

I am not sure what you mean by this. Could you provide a small example outside of the context of Ray?

[ogrisel]

Additional question: do you want this for the functions / classes defined in the __main__ module that reference global variables from that same module only or something more generic?

[suquark]

@ogrisel infer global variables refers to the following code:

cloudpickle/cloudpickle/cloudpickle.py

Lines 643 to 649 in 2ba0847

	if base_globals is None:
	# For functions defined in a well behaved module use
	# vars(func.__module__) for base_globals. This is necessary to
	# share the global variables across multiple pickled functions from
	# this module.
	if hasattr(func, '__module__') and func.__module__ is not None:
	base_globals = func.__module__

cloudpickle/cloudpickle/cloudpickle.py

Lines 1105 to 1117 in 2ba0847

	elif isinstance(base_globals, str):
	base_globals_name = base_globals
	try:
	# First try to reuse the globals from the module containing the
	# function. If it is not possible to retrieve it, fallback to an
	# empty dictionary.
	base_globals = vars(importlib.import_module(base_globals))
	except ImportError:
	base_globals = _dynamic_modules_globals.get(
	base_globals_name, None)
	if base_globals is None:
	base_globals = _DynamicModuleFuncGlobals()
	_dynamic_modules_globals[base_globals_name] = base_globals

We found it could get a wrong reference of itself when serializing a recursive function under some complex settings. (We further hacked the recursive call by assign the function itself to its globals after deserialization.)

[ogrisel]

This should be fixed by #240.

[ogrisel]

The fix is now available in cloudpickle 0.8.0 on PyPI.