Skip to content

Fix Nemo#2173 #2216

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.

Sign up for GitHub

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

Jump to bottom
Merged
joschmitt merged 2 commits into from
Nov 20, 2025
Merged

Fix Nemo#2173 #2216

joschmitt merged 2 commits into from
Nov 20, 2025

Conversation

@joschmitt
Copy link
Collaborator

@joschmitt joschmitt commented Nov 18, 2025

Closes Nemocas/Nemo.jl#2173
See Nemocas/Nemo.jl#2173 (comment) for why this is necessary.

@joschmitt joschmitt added bugfix This change fixes an existing bug matrices release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Nov 18, 2025
lgoettgens
lgoettgens approved these changes Nov 18, 2025
View reviewed changes
Copy link
Member

@lgoettgens lgoettgens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks reasonable!

@joschmitt joschmitt enabled auto-merge (squash) November 18, 2025 13:27
@lgoettgens
Copy link
Member

lgoettgens commented Nov 18, 2025

The CI failure is due to oscar-system/Oscar.jl#5562

@joschmitt joschmitt disabled auto-merge November 18, 2025 16:04
@joschmitt
Copy link
Collaborator Author

joschmitt commented Nov 18, 2025

Good, good.

@thofma
Copy link
Member

thofma commented Nov 18, 2025

It is a bit odd to do inv via solving for these matrices. Wouldn't it be more "natural" to delegate inv to the inv of the corresponding matrix space element? (Once we have better MatRingElem)

@joschmitt
Copy link
Collaborator Author

joschmitt commented Nov 19, 2025

Makes sense. Should the can_solve_with_solution then go completely?

@thofma
Copy link
Member

thofma commented Nov 19, 2025

Doing it properly via inv also needs #2207, so we can merge this here.

@joschmitt joschmitt enabled auto-merge (squash) November 20, 2025 13:25
@joschmitt joschmitt merged commit 5371d6c into Nemocas:master Nov 20, 2025
26 of 40 checks passed
@joschmitt joschmitt deleted the js/inv branch November 21, 2025 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lgoettgens lgoettgens lgoettgens approved these changes

Assignees

No one assigned

Labels

bugfix This change fixes an existing bug matrices release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Can't invert an element of a matrix ring over an AcbField

3 participants

@joschmitt @lgoettgens @thofma