gap-packages / qpa

GAP package for quivers and path algebras
https://folk.ntnu.no/oyvinso/QPA/
GNU General Public License v2.0
30 stars 13 forks source link

NC versions of PreImages... #78

Closed cdwensley closed 1 year ago

cdwensley commented 1 year ago

PreImages, PreImagesElm, PreImagesSet and PreImagesRepresentative, can all return incorrect results when the element(s) supplied are not in the range of the map. This situation has been discussed in GAP issue #4809. To rectify the situation the plan is to have NC versions of these four operations and to add tests to the non-NC versions. The procedure to be adopted is as follows. (1) Rename the four operations by adding 'NC' to their names, and then declare the original operations as synonyms of these. PR #5073 addresses this. (2) Ask package authors/maintainers to convert all their calls to these functions to the NC versions (unless there is good reason not to do so). A clause added to init.g ensures that the package still works in older versions of GAP. (3) Once all the packages have been updated, add tests to the non-NC versions of the operations. In the case of qpa, the method installed specifically checks that the element is in the range, so it appears that the change to NC should not be made in this package. Please confirm.

sunnyquiver commented 1 year ago

Only PreImagesRepresentative is implemented in QPA, and this implementation checks if the element entered into the function is in the range of the entered homomorphism. So it seems to me that the change don't apply to QPA.

cdwensley commented 1 year ago

Thanks for the confirmation. If (and it's a big if) PR #5073 ever gets merged it might be worth revisiting this issue.