radical of a zero dimensional assoc. algebra #1701
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
is the algebra itself; up to now, GAP ran into an error. (I have seen code that had to work around this error.)
markuspf
reviewed
Sep 8, 2017
| @@ -133,6 +133,8 @@ gap> radc:= RadicalOfAlgebra( c ); | |||
| <algebra of dimension 0 over CF(5)> | |||
| gap> Dimension( radc ); | |||
| 0 | |||
| gap> Dimension( RadicalOfAlgebra( radc ) ); | |||
| 0 | |||
There was a problem hiding this comment.
Maybe for sanity check also RadicalOfAlgebra(radc) = radc?
There was a problem hiding this comment.
I think that's kind of automatic if both are zero-dimensional?
Codecov Report
@@ Coverage Diff @@
## master #1701 +/- ##
==========================================
+ Coverage 64.42% 64.42% +<.01%
==========================================
Files 1002 1002
Lines 326705 326866 +161
Branches 13216 13216
==========================================
+ Hits 210480 210590 +110
- Misses 113358 113405 +47
- Partials 2867 2871 +4
|
olexandr-konovalov
added
the
gapdays2017-fall
fingolfin
approved these changes
Sep 10, 2017
| @@ -133,6 +133,8 @@ gap> radc:= RadicalOfAlgebra( c ); | |||
| <algebra of dimension 0 over CF(5)> | |||
| gap> Dimension( radc ); | |||
| 0 | |||
| gap> Dimension( RadicalOfAlgebra( radc ) ); | |||
| 0 | |||
There was a problem hiding this comment.
I think that's kind of automatic if both are zero-dimensional?
| @@ -2736,6 +2736,11 @@ InstallMethod( RadicalOfAlgebra, | |||
| fi; | |||
|
|
|||
| n:= Dimension( A ); | |||
|
|
|||
| if n = 0 then | |||
| return A; | |||
There was a problem hiding this comment.
Should it also set ParentAttr to A? It is bound in radicals returned by SubalgebraNC( A, rad, "basis" ).
There was a problem hiding this comment.
Setting a parent is useful only if exploiting the parent relation yields some information, thus setting an object to its own parent should be avoided.
fingolfin
added
kind: bug
olexandr-konovalov
added
the
release notes: added
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The radical of a zero dimensional assoc. algebra is the algebra itself.
Up to now, GAP ran into an error.
(I have seen code that had to work around this error.)