-
Notifications
You must be signed in to change notification settings - Fork 6
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
use ACE 4.1 #36
base: master
Are you sure you want to change the base?
use ACE 4.1 #36
Conversation
The test
hitting ^C gives
Probably another ACE output parsing bug. |
todo:
|
statistics package was removed from ACE 4. "options;" ACE command produces different output in diffent state of the enumerator - perhaps, we can just remove ACEBinaryVersion all together
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #36 +/- ##
==========================================
- Coverage 38.82% 38.46% -0.37%
==========================================
Files 10 10
Lines 2761 2704 -57
==========================================
- Hits 1072 1040 -32
+ Misses 1689 1664 -25
|
All what
and it breaks if there was some activity enumerating cosets before - due to a parsing error of the output. |
reverting this commit will re-surface an ACE4-specific bug, which makes ACEBinaryVersion hang if there is an active ACE session.
Removed ACE command in 4.1 to take care of:
|
Changed in ACE4 commands:
So our change (to always call |
It all works, except that the option aceiputfile to write the ACE input in an external file writes there the line aceinputile: <file name> - which ACE4 does not understand, but happily ignores. So this is a minor quirk.
1) "a set of figures detailing the control flow in the state machine that is at the core of the enumerator (see the enum.c & enum0n.inc source files)." 2) example demonstrating new functionality of "pr;" command: "The print table now takes two arguments, the beginning and ending coset. If the arguments are negative they mean (respectively) add order/representative columns to the table and include coincident coset rows. To compact and standardise the coset table you need to invoke st; before pr;."
Anyhow, this is working, the only issue here to resolve is that the docbuld action lives in my GitHub fork. |
No need for a special action. You can just insert a step which installs the "missing" packages. E.g. https://github.com/gap-packages/polymaking/blob/master/.github/workflows/CI.yml does this to install polymake |
I've modified https://github.com/dimpase/build-pkg-docs/blob/patch-1/action.yml so that installing xetex and metapost is optional, and doesn't happen by default. If this is too special to your taste, I can probably change it so that optionally |
OK, now d8382f5 does what you suggest. (I feel like yaml is slowly swapping in my brain to take place of some other language in cache 🥲 ) |
some commits can certainly be squashed if needed |
? Yes ? What ? |
Please review if possible |
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 have no objection and suggest we merge these changes, but do not have time to check details.
I'm asking Leonard @lhsoicher to test this. |
@gregg0 - are you still following this? |
Dear Dima,
I don’t understand what’s being done.
Is there something that explains it?
Regards,
Greg
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Dima Pasechnik ***@***.***>
Sent: Tuesday, February 6, 2024 8:16:37 PM
To: gap-packages/ace ***@***.***>
Cc: Gregory Gamble ***@***.***>; Mention ***@***.***>
Subject: Re: [gap-packages/ace] use ACE 4.1 (PR #36)
@gregg0<https://github.com/gregg0> - are you still following this?
—
Reply to this email directly, view it on GitHub<#36 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABKQEEH2JD252AFDKSVUSBTYSINKLAVCNFSM6AAAAAAVYNPIR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRZGQYDANBRG4>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Colin released an update to ACE, and I used it to update the package, adding documentation building along the way. |
Dear Dima,
I always found the way github did things hard to get started with, and was so grateful when Max got involved.
But it looks like I really have to get my head around it now,
because other things - websites - I'm involved with have moved to github.
Anyway, I trust your work ... so I won't hold you up,
and I'll try to look at what your work soon.
Regards,
Greg
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Dima Pasechnik ***@***.***>
Sent: Tuesday, February 6, 2024 11:50:25 PM
To: gap-packages/ace ***@***.***>
Cc: Gregory Gamble ***@***.***>; Mention ***@***.***>
Subject: Re: [gap-packages/ace] use ACE 4.1 (PR #36)
Colin released an update to ACE, and I used it to update the package, adding documentation building along the way.
—
Reply to this email directly, view it on GitHub<#36 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABKQEEASEVW7DRDMBUPKNKDYSJGMDAVCNFSM6AAAAAAVYNPIR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZQGA4TKOJXGI>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
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 is a huge change, I don't have time to review it properly right now. Just some quick comments, though
# Create PDF version | ||
pdftex manual; pdftex manual | ||
xetex manual |
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 switch from tex and pdftex to xetex?
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.
well, xetex
is faster, can take unicode, and produces smaller pdf
files. And more modern, too. The better question IMHO is why GAP stays on the old tech :-)
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 also spent time (using xelatex) adjusting graphics generated in the docs by metapost. And changed manual to use modern graphics stack, and not some outdated postscript-based stuff. I don't think it's necessary to re-do this on the old TeX stack.
PS. Back in Nov 2023 you didn't question the need for xetex, see #36 (comment)
Co-authored-by: Max Horn <[email protected]>
Co-authored-by: Max Horn <[email protected]>
Co-authored-by: Max Horn <[email protected]>
Thanks for the review. I'll continue updating tomorrow. :-) |
OK, I think I've addressed everything. And marked what I thought as clear as resolved (perhaps I should not have?) I hope you don't mind xetex. |
well, what can I do to get this reviewed? |
this will fix #8