Opened 2 years ago

Last modified 15 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: sage-8.2
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) Commit: 80e144cc2532e88d9c19792c37c7d972e11aec4c
Dependencies: #23978 Stopgaps:

Description (last modified by sbrandhorst)

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

comment:1 Changed 2 years ago by sbrandhorst

  • Branch set to u/sbrandhorst/allow_rational_inner_products_for_free_quadratic_zz_modules

comment:2 Changed 2 years ago by sbrandhorst

  • Commit set to 87db5c08843acbfe05e49be1ad6dac515cd86672
  • Description modified (diff)
  • Status changed from new to needs_review
  • Summary changed from Allow rational inner_products for free quadratic ZZ-modules to allow 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 2 years ago by sbrandhorst

  • 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 22 months ago by git

  • Commit changed from 87db5c08843acbfe05e49be1ad6dac515cd86672 to da0efa3cbd28c6afcc2ea6b0bf2e29debc159a7b

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 22 months ago by git

  • Commit changed from da0efa3cbd28c6afcc2ea6b0bf2e29debc159a7b to 1182776c2fcb849fa47b184569804f4ccb426403

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

1182776Changed printing. restriced inner_product_ring

comment:6 Changed 22 months ago by sbrandhorst

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

comment:7 Changed 21 months ago by git

  • Commit changed from 1182776c2fcb849fa47b184569804f4ccb426403 to 490980cafa15c02f435cf5d554211ba7e1312d13

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 21 months ago by sbrandhorst

  • Authors set to Simon Brandhorst
  • Milestone changed from sage-8.1 to sage-8.2

comment:9 Changed 20 months ago by tscrim

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 20 months ago by tscrim (previous) (diff)

comment:10 Changed 19 months ago by sbrandhorst

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 19 months ago by sbrandhorst

  • Status changed from needs_review to needs_work

comment:12 Changed 19 months ago by git

  • Commit changed from 490980cafa15c02f435cf5d554211ba7e1312d13 to 80e144cc2532e88d9c19792c37c7d972e11aec4c

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 19 months ago by sbrandhorst

  • Status changed from needs_work to needs_review

Is this what you had in mind?

comment:14 Changed 19 months ago by tscrim

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 follow-up: Changed 19 months ago by sbrandhorst

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

comment:16 in reply to: ↑ 15 Changed 19 months ago by tscrim

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 15 months ago by sbrandhorst

  • Status changed from needs_review to needs_work

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

Note: See TracTickets for help on using tickets.