Opened 4 years ago
Last modified 3 months ago
#23958 needs_work enhancement
allow inner products of free modules to take values in rings different from the base ring.
Reported by:  sbrandhorst  Owned by:  

Priority:  major  Milestone:  sage9.4 
Component:  linear algebra  Keywords:  sd91 
Cc:  roed  Merged in:  
Authors:  Simon Brandhorst  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  u/sbrandhorst/allow_rational_inner_products_for_free_quadratic_zz_modules (Commits, GitHub, GitLab)  Commit:  80e144cc2532e88d9c19792c37c7d972e11aec4c 
Dependencies:  #23978  Stopgaps: 
Description (last modified by )
Inner products / bilinear forms like
ZZ x ZZ > CC , RR, QQ
etc. make sense.
More general, let M
be a free module over R
and let B
be a ring containing R
then inner products
M x M > B
make sense and can be allowed.
Change History (20)
comment:1 Changed 4 years ago by
 Branch set to u/sbrandhorst/allow_rational_inner_products_for_free_quadratic_zz_modules
comment:2 Changed 4 years ago by
 Commit set to 87db5c08843acbfe05e49be1ad6dac515cd86672
 Description modified (diff)
 Status changed from new to needs_review
 Summary changed from Allow rational inner_products for free quadratic ZZmodules to allow inner products of free modules to take values in rings different from the base ring.
comment:3 Changed 4 years ago by
 Dependencies set to #23978
 Status changed from needs_review to needs_work
If we bring the inner_product_ring into play, we have to use it for comparison as well. So first we should implement #23978
comment:4 Changed 3 years ago by
 Commit changed from 87db5c08843acbfe05e49be1ad6dac515cd86672 to da0efa3cbd28c6afcc2ea6b0bf2e29debc159a7b
comment:5 Changed 3 years ago by
 Commit changed from da0efa3cbd28c6afcc2ea6b0bf2e29debc159a7b to 1182776c2fcb849fa47b184569804f4ccb426403
Branch pushed to git repo; I updated commit sha1. New commits:
1182776  Changed printing. restriced inner_product_ring

comment:6 Changed 3 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
comment:7 Changed 3 years ago by
 Commit changed from 1182776c2fcb849fa47b184569804f4ccb426403 to 490980cafa15c02f435cf5d554211ba7e1312d13
Branch pushed to git repo; I updated commit sha1. New commits:
490980c  Merge branch 'develop' into t/23958/allow_rational_inner_products_for_free_quadratic_zz_modules

comment:8 Changed 3 years ago by
 Milestone changed from sage8.1 to sage8.2
comment:9 Changed 3 years ago by
I don't think this is scaling well. IMO, it would be better to have a class for inner products that holds its (co)domains and takes that into account for comparisons. This would have better separationofconcerns and make the code easier to maintain.
Addendum: I might also have a use for such a class; plus it would allow you to use coercions.
comment:10 Changed 3 years ago by
Maybe you are right. I guess calling the coercion model each time you compute the inner product is too slow. So explicitly saving the ring might be better. Though it might be a while till I have time for this.
comment:11 Changed 3 years ago by
 Status changed from needs_review to needs_work
comment:12 Changed 3 years ago by
 Commit changed from 490980cafa15c02f435cf5d554211ba7e1312d13 to 80e144cc2532e88d9c19792c37c7d972e11aec4c
comment:13 Changed 3 years ago by
 Status changed from needs_work to needs_review
Is this what you had in mind?
comment:14 Changed 3 years ago by
Somewhat. I was thinking of having a full class for bilinear forms along the lines of:
class BilinearForm(object): def __init__(self, V, W, R, matrix): self._matrix = matrix self._V = V self._W = W self._codomain = R def __call__(self, v, w): v = self._V(v) w = self._W(w) return self._codomain(dot_product_wrt_matrix(v, w)) def codomain(self): return self._codomain
Ideally it would inherit from Morphism
, but we don't have tensor products of FreeModule
s.
Also, I believe only the first check for a coercion between two objects should be slow and all other subsequent coercion checks between the same two parents are cached.
comment:15 followup: ↓ 16 Changed 3 years ago by
This sounds like an almost complete rewrite of the FreeQuadraticModule
class?
comment:16 in reply to: ↑ 15 Changed 3 years ago by
Replying to sbrandhorst:
This sounds like an almost complete rewrite of the
FreeQuadraticModule
class?
It shouldn't be. It is just extracting out one component: the bilinear form (matrix). However, you are putting more and more into one class that is not purely a concern for that class. Does the FreeQuadraticModule
class care about the inner product ring or is that really a concern of the bilinear form? Think about it this way, mathematically you first define a bilinear form, then you define the quadratic module, correct?
Also, it would make it easier for other code that uses inner products to take advantage. In principle, all you should be doing is just wrapping the inner_product_matrix
object, so there should be minimal change beyond that (maybe a few other redirects).
comment:17 Changed 3 years ago by
 Status changed from needs_review to needs_work
Guess you are right. Though in the near future I will not have time for it.
comment:18 Changed 10 months ago by
 Milestone changed from sage8.2 to sage9.2
comment:19 Changed 9 months ago by
 Milestone changed from sage9.2 to sage9.3
comment:20 Changed 3 months ago by
 Milestone changed from sage9.3 to sage9.4
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
New commits:
Inner product rings can now be specified.