Opened 14 months ago
Closed 12 months ago
#31658 closed enhancement (fixed)
Slight speedup in copy method
Reported by:  ghmjungmath  Owned by:  

Priority:  major  Milestone:  sage9.4 
Component:  manifolds  Keywords:  copy, tensorfields, sections 
Cc:  egourgoulhon, tscrim, mkoeppe  Merged in:  
Authors:  Michael Jung  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  ffa35d9 (Commits, GitHub, GitLab)  Commit:  ffa35d9380ca0a717c1b90ed8b9e7fbe878f7dea 
Dependencies:  Stopgaps: 
Description (last modified by )
We have for the copy method of tensor fields and sections currently the following code:
resu = self._new_instance() for dom, rst in self._restrictions.items(): resu._restrictions[dom] = rst.copy() resu.set_name(name=name, latex_name=latex_name) resu._is_zero = self._is_zero return resu
where resu.set_name(name=name, latex_name=latex_name)
loops over all restrictions again. These two loops can be reduced to just one loop by passing name
and latex_name
to the restriction's copymethod.
Change History (18)
comment:1 Changed 14 months ago by
 Branch set to u/ghmjungmath/copy_tensorfield
comment:2 Changed 14 months ago by
 Branch u/ghmjungmath/copy_tensorfield deleted
 Description modified (diff)
comment:3 Changed 14 months ago by
 Description modified (diff)
 Status changed from new to needs_review
comment:4 Changed 14 months ago by
 Branch set to u/ghmjungmath/copy_tensorfield
comment:5 Changed 14 months ago by
 Commit set to a56ee0d39b3ab4bfbd673d8d216502daab3fbc7e
 Reviewers set to Travis Scrimshaw
comment:6 Changed 14 months ago by
Although your commit message also leaves something to be desired too.
comment:7 Changed 14 months ago by
Oh yes. I actually wanted to amend the commit before pushing. :D
comment:8 Changed 14 months ago by
Green bot, but please amend the commit before setting to a positive review.
comment:9 Changed 14 months ago by
 Commit changed from a56ee0d39b3ab4bfbd673d8d216502daab3fbc7e to 27c2fcb3f19491570555506cbeca6aefbe3a40d2
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
27c2fcb  Trac #31658: pass names to branched copy method

comment:10 Changed 14 months ago by
 Status changed from needs_review to positive_review
There we go. Thank you for the review Travis!
comment:11 Changed 14 months ago by
 Commit changed from 27c2fcb3f19491570555506cbeca6aefbe3a40d2 to b0240cc16c1c08e5d7b3cf3c226d5b33151ec86e
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
b0240cc  Trac #31658: same change for copy_from

comment:12 Changed 14 months ago by
Just noticed, the same change can be applied to copy_from
making it faster, too. Sorry Travis!
comment:13 Changed 14 months ago by
 Status changed from needs_review to positive_review
comment:14 Changed 13 months ago by
 Commit changed from b0240cc16c1c08e5d7b3cf3c226d5b33151ec86e to ffa35d9380ca0a717c1b90ed8b9e7fbe878f7dea
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:
ffa35d9  Trac #31658: same change for copy_from

comment:15 Changed 13 months ago by
Typo in code caused pyflakes error.
comment:16 Changed 13 months ago by
 Status changed from needs_review to positive_review
comment:17 Changed 13 months ago by
 Milestone changed from sage9.3 to sage9.4
comment:18 Changed 12 months ago by
 Branch changed from u/ghmjungmath/copy_tensorfield to ffa35d9380ca0a717c1b90ed8b9e7fbe878f7dea
 Resolution set to fixed
 Status changed from positive_review to closed
Green bot => you can set a positive review.
New commits:
something