Opened 6 months ago

Closed 4 months ago

#31658 closed enhancement (fixed)

Slight speed-up in copy method

Reported by: gh-mjungmath Owned by:
Priority: major Milestone: sage-9.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:

Status badges

Description (last modified by gh-mjungmath)

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 copy-method.

Change History (18)

comment:1 Changed 6 months ago by gh-mjungmath

  • Branch set to u/gh-mjungmath/copy_tensorfield

comment:2 Changed 6 months ago by gh-mjungmath

  • Branch u/gh-mjungmath/copy_tensorfield deleted
  • Description modified (diff)

comment:3 Changed 6 months ago by gh-mjungmath

  • Description modified (diff)
  • Status changed from new to needs_review

comment:4 Changed 6 months ago by gh-mjungmath

  • Branch set to u/gh-mjungmath/copy_tensorfield

comment:5 Changed 6 months ago by tscrim

  • Commit set to a56ee0d39b3ab4bfbd673d8d216502daab3fbc7e
  • Reviewers set to Travis Scrimshaw

Green bot => you can set a positive review.


New commits:

a56ee0dsomething

comment:6 Changed 6 months ago by tscrim

Although your commit message also leaves something to be desired too.

comment:7 Changed 6 months ago by gh-mjungmath

Oh yes. I actually wanted to amend the commit before pushing. :D

comment:8 Changed 6 months ago by tscrim

Green bot, but please amend the commit before setting to a positive review.

comment:9 Changed 6 months ago by git

  • Commit changed from a56ee0d39b3ab4bfbd673d8d216502daab3fbc7e to 27c2fcb3f19491570555506cbeca6aefbe3a40d2

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

27c2fcbTrac #31658: pass names to branched copy method

comment:10 Changed 6 months ago by gh-mjungmath

  • Status changed from needs_review to positive_review

There we go. Thank you for the review Travis!

comment:11 Changed 6 months ago by git

  • 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:

b0240ccTrac #31658: same change for copy_from

comment:12 Changed 6 months ago by gh-mjungmath

Just noticed, the same change can be applied to copy_from making it faster, too. Sorry Travis!

comment:13 Changed 6 months ago by tscrim

  • Status changed from needs_review to positive_review

comment:14 Changed 6 months ago by git

  • 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:

ffa35d9Trac #31658: same change for copy_from

comment:15 Changed 6 months ago by gh-mjungmath

Typo in code caused pyflakes error.

comment:16 Changed 6 months ago by tscrim

  • Status changed from needs_review to positive_review

comment:17 Changed 6 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

comment:18 Changed 4 months ago by vbraun

  • Branch changed from u/gh-mjungmath/copy_tensorfield to ffa35d9380ca0a717c1b90ed8b9e7fbe878f7dea
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.