-
Notifications
You must be signed in to change notification settings - Fork 19
Improve and unify names (3) #289
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
base: master
Are you sure you want to change the base?
Conversation
RecognitionInfoFamily -> RecogNodeFamily IsRecognitionInfo -> IsRecogNode RIFac -> ImageRecogNode RIKer -> KernelRecogNode
| gap tst/testall.g | ||
|
|
||
| On failure you can always roll back to the last commit, i.e. the version before | ||
| applying the renamings. For example one could type in the command `git reset --hard`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| applying the renamings. For example one could type in the command `git reset --hard`. | |
| applying the renamings. For example one could execute the command `git reset --hard`. |
| list := reverse(Set(list)); | ||
| return list; | ||
| fi; # largest prime factor of d-2 | ||
| fi; # largest prime image of d-2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose here the renaming script made a mistake?
| #I Going to the image (depth=0, try=1). | ||
| #I Finished rank 95 method "MovesOnlySmallPoints": success. | ||
| #I Back from factor (depth=0). | ||
| #I Back from image (depth=0). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a mistake as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, perhaps not. I'm not sure.
| # since we know exactly what kind of group we are looking | ||
| # at, we don't want to run generic recognition on the | ||
| # factor group and the kernel. So we provide "hints" to | ||
| # image group and the kernel. So we provide "hints" to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, but this has to be wrong. I don't know what an "image group" should be.
| might use another database of recognition methods because the | ||
| homomorphism might change the representation of the group.</Item> | ||
| <Item>After successful recognition of the factor group the procedure has | ||
| <Item>After successful recognition of the image group the procedure has |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Image group" again.
| <Item>Write the element in the image group as a product of the nice | ||
| generators in the image group.</Item> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again "image group".
| changes the representation (for example going from matrix groups to | ||
| permutation groups), you will have to set the attribute | ||
| <Ref Attr="methodsforfactor"/> accordingly. Also, | ||
| <Ref Attr="methodsforimage"/> accordingly. Also, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be camel-case as well?
| ## function <Ref Func="CalcNiceGensGeneric"/> installed in the | ||
| ## <Ref Attr="calcnicegens"/> attribute. | ||
| ## function <Ref Func="CalcNiceGeneratorsForLeafNode"/> installed in the | ||
| ## <Ref Attr="RECOG_CalcNiceGeneratorsFunctionOfRecogNode"/> attribute. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the RECOG_ prefix?
|
Looking through this I found several things I don't fully understand. Somehow every occurrence of "factor" is replace by "image". Somethings renamed are renamed to a name which is still not in camel case and some new names are prefix with "RECOG_". Is there a particular reason for this? In general, this should be restructured to first add the script and then perform the actual renamings. Adding the renamings to the docs could also be done manually as we don't have to rename that much anymore. Some things have been renamed already over the past years. So, the script could be simplified to just support renaming, probably even rewritten in something like shell or perl. |
Draft PR.