Opened 2 years ago

Closed 2 years ago

#20262 closed enhancement (fixed)

Add point transformation matrix for projective space.

Reported by: rlmiller Owned by: rlmiller
Priority: minor Milestone: sage-7.2
Component: algebraic geometry Keywords:
Cc: Merged in:
Authors: Rebecca Lauren Miller Reviewers: Ben Hutz
Report Upstream: N/A Work issues:
Branch: 010fa7b (Commits) Commit: 010fa7b72b59f0eb0077bce1a7e5c78988b1b61f
Dependencies: Stopgaps:

Description

Given two sets of n+2 points given that no n+1 subsets are linearly dependent, find the unique transformation matrix that maps one set to the other.

Change History (15)

comment:1 Changed 2 years ago by rlmiller

  • Branch set to u/rlmiller/moduli

comment:2 Changed 2 years ago by git

  • Commit set to f5c19931914cb0fa8311d2c18cedafb7b4450fec

Branch pushed to git repo; I updated commit sha1. New commits:

f5c199320262 fixed white space

comment:3 Changed 2 years ago by rlmiller

  • Status changed from new to needs_review

comment:4 Changed 2 years ago by bhutz

  • Reviewers set to Ben Hutz
  • Status changed from needs_review to needs_work

A couple minor things here

  • need to do input checking on both the codomains of the points and the number of points entered
P1.<a,b,c>=ProjectiveSpace(QQ, 2)
P.<x,y,z>=ProjectiveSpace(QQ, 2)
points_source=[P1([1,4,1]),P([1,2,2]),P1([3,5,1]),P1([1,-1,1])]
points_target=[P([5,-2,7]),P([3,-2,3]),P([6,-5,9]), P([3,6,7])]
P1.<a,b,c>=ProjectiveSpace(QQ, 2)
points_source=[P1([1,4,1]),P1([1,2,2]),P1([3,5,1]),P1([13,5,-1]),P1([3,25,4])]
points_target=[P1([5,-2,7]),P1([3,-2,3]),P1([6,-5,9]), P1([3,6,7])]
P1.point_transformation_matrix(points_source, points_target)
  • for the lines
w = v.rational_points(bound=n)
if len(w) == 0:
    raise ValueError (" no conjugation found, over the %s" %r)

you should remove the bound=n, and I don't think you need the if/raise as if you pass the linearly independent check you will have a unique solution defined over the base_ring

comment:5 Changed 2 years ago by git

  • Commit changed from f5c19931914cb0fa8311d2c18cedafb7b4450fec to 2d49d8b9456f93d0a018071ada84a3d9d0630d9c

Branch pushed to git repo; I updated commit sha1. New commits:

2d49d8b20262 Point transformation matrix, Added new value errors and updated code

comment:6 Changed 2 years ago by rlmiller

  • Status changed from needs_work to needs_review

Added new value errors and doctests for each new value error. Edited code as suggested.

comment:7 Changed 2 years ago by bhutz

  • Status changed from needs_review to needs_work

Functionality looks fine, just two documentation things.

  • The description of the function needs improvement.
  • one of the examples is missing a blank line after the :: so is not formatting correctly

comment:8 Changed 2 years ago by git

  • Commit changed from 2d49d8b9456f93d0a018071ada84a3d9d0630d9c to 293415c731d3e1e7b991bdf7ee897c1ad80eb3be

Branch pushed to git repo; I updated commit sha1. New commits:

293415c20262 Point transformation matrix- fixed style and updated description

comment:9 Changed 2 years ago by rlmiller

  • Status changed from needs_work to needs_review

comment:10 Changed 2 years ago by git

  • Commit changed from 293415c731d3e1e7b991bdf7ee897c1ad80eb3be to 2c82468e33679ba6be6a72a64cde1fba7ed6e731

Branch pushed to git repo; I updated commit sha1. New commits:

2c8246820262 Point Transformation Matrix -Updated documentation

comment:11 Changed 2 years ago by git

  • Commit changed from 2c82468e33679ba6be6a72a64cde1fba7ed6e731 to c74ec51fd0028aae3e4410a5de511d1c8dfcc309

Branch pushed to git repo; I updated commit sha1. New commits:

c74ec5120262 Point Transformation Matrix- Updated documentation

comment:12 Changed 2 years ago by bhutz

err..you mean no n+1 are linearly *dependent*. I'd just fix it, but I have this pulled into another branch....

comment:13 Changed 2 years ago by git

  • Commit changed from c74ec51fd0028aae3e4410a5de511d1c8dfcc309 to 010fa7b72b59f0eb0077bce1a7e5c78988b1b61f

Branch pushed to git repo; I updated commit sha1. New commits:

010fa7b20262 Point Transformation Matrix- Fixed typo in documentation

comment:14 Changed 2 years ago by bhutz

  • Status changed from needs_review to positive_review

comment:15 Changed 2 years ago by vbraun

  • Branch changed from u/rlmiller/moduli to 010fa7b72b59f0eb0077bce1a7e5c78988b1b61f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.