Opened 5 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: Simon Brandhorst Owned by:
Priority: major Milestone: sage-9.8
Component: linear algebra Keywords: sd91
Cc: David Roe 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:

Status badges

Description (last modified by Simon Brandhorst)

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 (24)

comment:1 Changed 5 years ago by Simon Brandhorst

Branch: u/sbrandhorst/allow_rational_inner_products_for_free_quadratic_zz_modules

comment:2 Changed 5 years ago by Simon Brandhorst

Commit: 87db5c08843acbfe05e49be1ad6dac515cd86672
Description: modified (diff)
Status: newneeds_review
Summary: Allow rational inner_products for free quadratic ZZ-modulesallow inner products of free modules to take values in rings different from the base ring.

New commits:

87db5c0Inner product rings can now be specified.

comment:3 Changed 5 years ago by Simon Brandhorst

Dependencies: #23978
Status: needs_reviewneeds_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 5 years ago by git

Commit: 87db5c08843acbfe05e49be1ad6dac515cd86672da0efa3cbd28c6afcc2ea6b0bf2e29debc159a7b

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

8f2172dMerge branch 'develop' into t/23958/allow_rational_inner_products_for_free_quadratic_zz_modules
da0efa3Added inner_product_ring

comment:5 Changed 5 years ago by git

Commit: da0efa3cbd28c6afcc2ea6b0bf2e29debc159a7b1182776c2fcb849fa47b184569804f4ccb426403

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

1182776Changed printing. restriced inner_product_ring

comment:6 Changed 5 years ago by Simon Brandhorst

Description: modified (diff)
Status: needs_workneeds_review

comment:7 Changed 5 years ago by git

Commit: 1182776c2fcb849fa47b184569804f4ccb426403490980cafa15c02f435cf5d554211ba7e1312d13

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

490980cMerge branch 'develop' into t/23958/allow_rational_inner_products_for_free_quadratic_zz_modules

comment:8 Changed 5 years ago by Simon Brandhorst

Authors: Simon Brandhorst
Milestone: sage-8.1sage-8.2

comment:9 Changed 5 years ago by Travis Scrimshaw

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 separation-of-concerns 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.

Last edited 5 years ago by Travis Scrimshaw (previous) (diff)

comment:10 Changed 5 years ago by Simon Brandhorst

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 5 years ago by Simon Brandhorst

Status: needs_reviewneeds_work

comment:12 Changed 5 years ago by git

Commit: 490980cafa15c02f435cf5d554211ba7e1312d1380e144cc2532e88d9c19792c37c7d972e11aec4c

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

06e28a5Merge branch 'develop' into t/23958/allow_rational_inner_products_for_free_quadratic_zz_modules
80e144cinner_product_ring as attribute

comment:13 Changed 5 years ago by Simon Brandhorst

Status: needs_workneeds_review

Is this what you had in mind?

comment:14 Changed 5 years ago by Travis Scrimshaw

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

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 Changed 5 years ago by Simon Brandhorst

This sounds like an almost complete rewrite of the FreeQuadraticModule class?

comment:16 in reply to:  15 Changed 5 years ago by Travis Scrimshaw

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 4 years ago by Simon Brandhorst

Status: needs_reviewneeds_work

Guess you are right. Though in the near future I will not have time for it.

comment:18 Changed 2 years ago by Matthias Köppe

Milestone: sage-8.2sage-9.2

comment:19 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-9.3

comment:20 Changed 22 months ago by Matthias Köppe

Milestone: sage-9.3sage-9.4

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

comment:21 Changed 17 months ago by Matthias Köppe

Milestone: sage-9.4sage-9.5

Setting a new milestone for this ticket based on a cursory review.

comment:22 Changed 12 months ago by Matthias Köppe

Milestone: sage-9.5sage-9.6

comment:23 Changed 8 months ago by Matthias Köppe

Milestone: sage-9.6sage-9.7

comment:24 Changed 3 months ago by Matthias Köppe

Milestone: sage-9.7sage-9.8
Note: See TracTickets for help on using tickets.