From 5be7e0be58728e164d180a5d0f689177698d174f Mon Sep 17 00:00:00 2001 From: bela127 Date: Mon, 23 Jan 2023 13:52:29 +0100 Subject: [PATCH 1/3] fix bug that prevents deepcopy and pickling if object has a parent --- paramz/core/gradcheckable.py | 4 ++-- paramz/core/parameter_core.py | 5 +++++ paramz/core/parentable.py | 29 +++++++++++++++++++++++++++-- paramz/core/pickleable.py | 15 ++++++++++----- 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/paramz/core/gradcheckable.py b/paramz/core/gradcheckable.py index 4cac3b4..42bc805 100644 --- a/paramz/core/gradcheckable.py +++ b/paramz/core/gradcheckable.py @@ -28,10 +28,9 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #=============================================================================== from . import HierarchyError -from .pickleable import Pickleable from .parentable import Parentable -class Gradcheckable(Pickleable, Parentable): +class Gradcheckable(Parentable): """ Adds the functionality for an object to be gradcheckable. It is just a thin wrapper of a call to the highest parent for now. @@ -74,3 +73,4 @@ def _checkgrad(self, param, verbose=0, step=1e-6, tolerance=1e-3, df_tolerance=1 TODO: this can be done more efficiently, when doing it inside here """ raise HierarchyError("This parameter is not in a model with a likelihood, and, therefore, cannot be gradient checked!") + diff --git a/paramz/core/parameter_core.py b/paramz/core/parameter_core.py index 34dd360..32324bd 100644 --- a/paramz/core/parameter_core.py +++ b/paramz/core/parameter_core.py @@ -491,6 +491,11 @@ def _name_changed(self, param, old_name): self._remove_parameter_name(None, old_name) self._add_parameter_name(param) + def __getstate__(self): + dc = super().__getstate__() + del dc['_param_array_'] + return dc + def __setstate__(self, state): super(Parameterizable, self).__setstate__(state) self.logger = logging.getLogger(self.__class__.__name__) diff --git a/paramz/core/parentable.py b/paramz/core/parentable.py index 2d8d9ed..34de026 100644 --- a/paramz/core/parentable.py +++ b/paramz/core/parentable.py @@ -27,8 +27,9 @@ # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #=============================================================================== +from .pickleable import Pickleable -class Parentable(object): +class Parentable(Pickleable): """ Enable an Object to have a parent. @@ -37,8 +38,9 @@ class Parentable(object): """ _parent_ = None _parent_index_ = None + def __init__(self, *args, **kwargs): - super(Parentable, self).__init__() + super().__init__(*args, **kwargs) def has_parent(self): """ @@ -73,3 +75,26 @@ def _notify_parent_change(self): Dont do anything if in leaf node """ pass + + def __getstate__(self): + dc = super().__getstate__() + dc.pop('_parent_', None) + return dc + + def __deepcopy__(self, memo: dict): + s = self.__new__(self.__class__) # fresh instance + memo[id(self)] = s # be sure to break all cycles --> self is already done + # The above line can cause hard to understand exceptions/bugs, because s is not 'done' its state attributes need to be copied first + # If a state attribute has a link to a parent object, the parent is copied first, using the uninitialized copy s + # thereby throwing an exception when attributes of the child are accessed while copying the parent. + # so a subclass should not link to its parent or handel the link like its done here. + import copy + + parent_copy = memo.get(id(self._parent_), None) #get the copy of the parent (it should already be in the memo), + # if the parent is not in the memo the copy process was not started from the parent, and the link should will be removed. + state = self.__getstate__() + updated_state = copy.deepcopy(state, memo) # standard copy + s.__setstate__(updated_state) + s._parent_ = parent_copy + return s + diff --git a/paramz/core/pickleable.py b/paramz/core/pickleable.py index faf45de..b38de52 100644 --- a/paramz/core/pickleable.py +++ b/paramz/core/pickleable.py @@ -84,7 +84,7 @@ def copy(self, memo=None, which=None): parents = [] if which is None: which = self - which.traverse_parents(parents.append) # collect parents + which.traverse_parents(parents.append) # collect parents #TODO: Refactor: This is bad, in this class we do not know anything about parentables for p in parents: if not id(p) in memo :memo[id(p)] = None # set all parents to be None, so they will not be copied if not id(self.gradient) in memo:memo[id(self.gradient)] = None # reset the gradient @@ -96,13 +96,18 @@ def copy(self, memo=None, which=None): def __deepcopy__(self, memo): s = self.__new__(self.__class__) # fresh instance - memo[id(self)] = s # be sure to break all cycles --> self is already done + memo[id(self)] = s # be sure to break all cycles --> self will be done after all children are done + # children should not have a link to parent as parent cant finish before children. import copy - s.__setstate__(copy.deepcopy(self.__getstate__(), memo)) # standard copy + + state = self.__getstate__() + updated_state = copy.deepcopy(state, memo) # standard copy + + s.__setstate__(updated_state) return s - def __getstate__(self): - ignore_list = ['_param_array_', # parameters get set from bottom to top + def __getstate__(self): #TODO: Refactor: This is bad, this class does not know about most of these attributes + ignore_list = [#'_param_array_', # parameters get set from bottom to top '_gradient_array_', # as well as gradients '_optimizer_copy_', 'logger', From 62d5d5c71a90c316a12a2abf0f5017740a5d95da Mon Sep 17 00:00:00 2001 From: bela127 Date: Mon, 6 Feb 2023 17:28:17 +0100 Subject: [PATCH 2/3] fix bug with deepcopy and pickle: New object has no atribute _param_array_ --- paramz/core/parameter_core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paramz/core/parameter_core.py b/paramz/core/parameter_core.py index 32324bd..5208a6d 100644 --- a/paramz/core/parameter_core.py +++ b/paramz/core/parameter_core.py @@ -493,7 +493,7 @@ def _name_changed(self, param, old_name): def __getstate__(self): dc = super().__getstate__() - del dc['_param_array_'] + dc.pop('_param_array_', None) return dc def __setstate__(self, state): From 2dd8deaf0087fb38227141147e5276794526fe8f Mon Sep 17 00:00:00 2001 From: bela127 Date: Fri, 17 Feb 2023 11:04:52 +0100 Subject: [PATCH 3/3] fix numpy deprecation np.float --- paramz/optimization/verbose_optimization.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paramz/optimization/verbose_optimization.py b/paramz/optimization/verbose_optimization.py index acef93f..9510aee 100644 --- a/paramz/optimization/verbose_optimization.py +++ b/paramz/optimization/verbose_optimization.py @@ -35,7 +35,7 @@ def exponents(fnow, current_grad): - exps = [np.abs(np.float(fnow)), + exps = [np.abs(np.float_(fnow)), 1 if current_grad is np.nan else current_grad] return np.sign(exps) * np.log10(exps).astype(int)