Issue 329 deepcopy #403
Issue 329 deepcopy #403
Conversation
Codecov Report
@@ Coverage Diff @@
## master #403 +/- ##
==========================================
+ Coverage 96.75% 96.81% +0.05%
==========================================
Files 60 60
Lines 3919 3928 +9
==========================================
+ Hits 3792 3803 +11
+ Misses 127 125 -2
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #403 +/- ##
==========================================
+ Coverage 96.46% 96.49% +0.02%
==========================================
Files 59 60 +1
Lines 3876 3879 +3
==========================================
+ Hits 3739 3743 +4
+ Misses 137 136 -1
Continue to review full report at Codecov.
|
pybamm/expression_tree/copy.py
Outdated
| import pybamm | ||
|
|
||
|
|
||
| def make_new_copy(symbol): |
There was a problem hiding this comment.
make_new_copy seems very similar in structure to simplify or evaluate, perhaps we should do this in a similar way. That is, have a new_copy function on Symbol, which is overridden on derived classes. You are already sorta doing that for the unary operators and concatenate anyway. What do you think?
There was a problem hiding this comment.
Yeah, it is a bit inconsistent at the moment. I was trying to keep it in line with the ParameterValues.process_symbol, Discretisation.process_symbol and simplify functions. The advantage of those is that we could later add a dictionary of already-processed symbols so that we avoid re-processing the same symbol multiple times. But maybe copying is straightforward enough that it should just be a function on Symbol.
There was a problem hiding this comment.
don't think we could use a dictionary of already-processed symbols here. Even if we've already copied a symbol, we still need to make another copy even if we find an identical symbol in the tree
There was a problem hiding this comment.
I think it would work, because what's causing problems with anytree is the parent attribute. So if we have a dict of variables whose parent is None, we can insert them into different places in the tree (since they get shallow-copied in Symbol.__init__) without having to deepcopy them again. As an aside, if we can find a better way of making a copy of an object in anytree with parent removed, then we don't need this whole thing
|
@martinjrobins - have now updated to your suggestion above |
…er-constant #403 add special case for constant function parameter
Description
Create function
make_new_copyto replacedeepcopyFixes #329
Type of change
Key checklist:
$ flake8$ python run-tests.py --unit$ cd docsand then$ make clean; make htmlYou can run all three at once, using
$ python run-tests.py --quick.Further checks: