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

Too short variable names #71

Open
olexandr-konovalov opened this issue Mar 10, 2020 · 10 comments
Open

Too short variable names #71

olexandr-konovalov opened this issue Mar 10, 2020 · 10 comments

Comments

@olexandr-konovalov
Copy link
Member

After loading, the following too short names starting with capitals are created:

[ "Fix", "TGS", "YB" ] 

This is not a very good practice, see https://www.gap-system.org/Manuals/doc/ref/chap76.html#X7DEACD9786DE29F1 - especially with such a trivial name like Fix.

@fingolfin
Copy link
Member

@vendramin even after your PR to fix the documentation for Fix, it is not quite clear to me why it is called like that?

@vendramin
Copy link
Collaborator

vendramin commented Aug 2, 2022 via email

@vendramin
Copy link
Collaborator

vendramin commented Aug 2, 2022 via email

@olexandr-konovalov
Copy link
Member Author

@vendramin GAP has some rules (or rather suggestions) indeed: https://docs.gap-system.org/doc/ref/chap76_mj.html#X7DEACD9786DE29F1. We need to think of some alternatives...

@olexandr-konovalov
Copy link
Member Author

@vendramin let's come back to this!

  1. Trying to understand what 'Fix' is actually returning. The manual says it returns an ideal, but it seems like returns a list. You said above that it's a set of of fixed points. Maybe FixedPointsOfSkewBrace?

  2. TGS is undocumented and defined here

    TGS:=[];

    Seems to me a global variable to store some data - could be called YangBaxterTGS or something. Could be also made write protected.

  3. YB is undocumented and defined here

    YangBaxter/lib/ybe.gi

    Lines 4 to 21 in 3958563

    ### This function returns the set-theoretic solution given by the permutations <l_actions> and <r_actions>
    ### <l_action> and <r_action> are matrices!
    InstallMethod(YB, "for a two lists of permutations", [ IsList, IsList ],
    function(l, r)
    local obj;
    if not IS_YB(l, r) then
    Error("this is not a solution of the YBE");
    fi;
    obj := Objectify(YBType, rec());
    SetSize(obj, Size(l));
    SetLMatrix(obj, List(l, x->List(x, y->y)));
    SetRMatrix(obj, List(r, x->List(x, y->y)));
    return obj;
    end);

    Maybe YBSolutionByPerms?

@vendramin
Copy link
Collaborator

vendramin commented May 18, 2023 via email

@olexandr-konovalov
Copy link
Member Author

  1. FixedPointsOfSkewBrace is ok then. My concerns is that it returns a list - so a different type of object, not the one promised in the manual (neither ideal nor left ideal)

  2. Yes, it's a data file which is read from tau.gd. So we can delete tau.* and data/TGsmall.g then?

  3. I would still go with YBSolutionByPerms - By convention is documented in GAP conventions, and Perms is used in various places as an abbreviation.

@vendramin
Copy link
Collaborator

vendramin commented May 18, 2023 via email

@olexandr-konovalov
Copy link
Member Author

  1. So then one should not just rename 1, but also fix the output - generate a left ideal by the list currently returns?
  2. If you remove them, they will still be recorded in the repository's revisions history though. If you have doubts, we can keep them, or move them to a branch instead of deleting... no hurry needed for that.
  3. ok, thanks!

@vendramin
Copy link
Collaborator

vendramin commented May 18, 2023 via email

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

3 participants