Opened 3 years ago

Closed 3 years ago

# Add point transformation matrix for projective space.

Reported by: Owned by: rlmiller rlmiller minor sage-7.2 algebraic geometry Rebecca Lauren Miller Ben Hutz N/A 010fa7b (Commits) 010fa7b72b59f0eb0077bce1a7e5c78988b1b61f

### 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.

### comment:1 Changed 3 years ago by rlmiller

• Branch set to u/rlmiller/moduli

### comment:2 Changed 3 years ago by git

• Commit set to f5c19931914cb0fa8311d2c18cedafb7b4450fec

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

 ​f5c1993 `20262 fixed white space`

### comment:3 Changed 3 years ago by rlmiller

• Status changed from new to needs_review

### comment:4 Changed 3 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 3 years ago by git

• Commit changed from f5c19931914cb0fa8311d2c18cedafb7b4450fec to 2d49d8b9456f93d0a018071ada84a3d9d0630d9c

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

 ​2d49d8b `20262 Point transformation matrix, Added new value errors and updated code`

### comment:6 Changed 3 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 3 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 3 years ago by git

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

 ​293415c `20262 Point transformation matrix- fixed style and updated description`

### comment:9 Changed 3 years ago by rlmiller

• Status changed from needs_work to needs_review

### comment:10 Changed 3 years ago by git

• Commit changed from 293415c731d3e1e7b991bdf7ee897c1ad80eb3be to 2c82468e33679ba6be6a72a64cde1fba7ed6e731

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

 ​2c82468 `20262 Point Transformation Matrix -Updated documentation`

### comment:11 Changed 3 years ago by git

• Commit changed from 2c82468e33679ba6be6a72a64cde1fba7ed6e731 to c74ec51fd0028aae3e4410a5de511d1c8dfcc309

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

 ​c74ec51 `20262 Point Transformation Matrix- Updated documentation`

### comment:12 Changed 3 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 3 years ago by git

• Commit changed from c74ec51fd0028aae3e4410a5de511d1c8dfcc309 to 010fa7b72b59f0eb0077bce1a7e5c78988b1b61f

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

 ​010fa7b `20262 Point Transformation Matrix- Fixed typo in documentation`

### comment:14 Changed 3 years ago by bhutz

• Status changed from needs_review to positive_review

### comment:15 Changed 3 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.