Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wrong result solving equation system with symbolic matrix #33159

Closed
dkrenn opened this issue Jan 13, 2022 · 17 comments
Closed

wrong result solving equation system with symbolic matrix #33159

dkrenn opened this issue Jan 13, 2022 · 17 comments

Comments

@dkrenn
Copy link
Contributor

dkrenn commented Jan 13, 2022

We get

sage: Matrix(SR, [[1, 1]]).solve_left(vector(SR, [2, 3]))                                                                                                    
(2)

which is wrong. This works for matrices over QQ as we can see by

sage: Matrix([[1, 1]]).solve_left(vector([2, 3]))                                                                                                            
ValueError: matrix equation has no solutions

CC: @mwageringel

Component: symbolics

Author: Michael Orlitzky

Branch: 0abe6bd

Reviewer: Daniel Krenn

Issue created by migration from https://trac.sagemath.org/ticket/33159

@dkrenn dkrenn added this to the sage-9.5 milestone Jan 13, 2022
@dkrenn
Copy link
Contributor Author

dkrenn commented Jan 13, 2022

comment:1

Wrong result on 9.5.rc1.

@orlitzky
Copy link
Contributor

comment:2

This is essentially #29729. Work stalled there because we don't have a solution for ball fields, but I think I can move my commit for SR to this ticket to fix this issue.

@orlitzky
Copy link
Contributor

New commits:

4d094d6Trac #33159: add special case to solve_right() for symbolic systems.

@orlitzky
Copy link
Contributor

Author: Michael Orlitzky

@orlitzky
Copy link
Contributor

Branch: u/mjo/ticket/33159

@orlitzky
Copy link
Contributor

Commit: 4d094d6

@dkrenn
Copy link
Contributor Author

dkrenn commented Jan 13, 2022

Reviewer: Daniel Krenn

@dkrenn
Copy link
Contributor Author

dkrenn commented Jan 13, 2022

Changed branch from u/mjo/ticket/33159 to u/dkrenn/ticket/33159

@dkrenn
Copy link
Contributor Author

dkrenn commented Jan 13, 2022

Changed commit from 4d094d6 to 0abe6bd

@dkrenn
Copy link
Contributor Author

dkrenn commented Jan 13, 2022

comment:6

Thank you for your patch and for doing this so fast. This looks almost good to me.

  1. I've added a small easy patch myself to handle subrings of the symbolic ring as well.
  2. I commented on the examples where no check is performed, so that it clearly states that the result is wrong in this case.

Everything else is fine for me. Patchbot not yet done. Please cross-review my changes.


New commits:

585dbcdTrac #33159: handle subrings of SR as well
0abe6bdTrac #33159: comment examples producing wrong output

@orlitzky
Copy link
Contributor

comment:7

Replying to @dkrenn:

Thank you for your patch and for doing this so fast. This looks almost good to me.

  1. I've added a small easy patch myself to handle subrings of the symbolic ring as well.
  2. I commented on the examples where no check is performed, so that it clearly states that the result is wrong in this case.

Both good ideas, thanks.

@dkrenn
Copy link
Contributor Author

dkrenn commented Jan 14, 2022

comment:8

Set to positive_review as everyone is happy and the patchbot as well (the failing of the docbuild plugin seems to be an issue of that particular patchbot (which was cleanly restarted a moment ago)).

@mwageringel
Copy link

comment:9

Thank you for resolving this.

@slel
Copy link
Member

slel commented Jan 30, 2022

comment:10

Setting milestone to 9.6 now that 9.5 is out.

@slel slel modified the milestones: sage-9.5, sage-9.6 Jan 30, 2022
@vbraun
Copy link
Member

vbraun commented Feb 13, 2022

Changed branch from u/dkrenn/ticket/33159 to 0abe6bd

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 21, 2022

Changed commit from 0abe6bd to none

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 21, 2022

comment:12

Follow-up in #33392

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants