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

Increase warning level for Conway polynomial #1363

Merged
merged 1 commit into from
May 29, 2017

Conversation

ChrisJefferson
Copy link
Contributor

This just increases the warning level of calculating conway polynomials. I don't feel this message is necessary for most users, and it is causing problems with some new tests I'm writing (and annoys me generally)

@codecov
Copy link

codecov bot commented May 26, 2017

Codecov Report

Merging #1363 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1363      +/-   ##
==========================================
+ Coverage   61.77%   61.77%   +<.01%     
==========================================
  Files        1035     1035              
  Lines      356668   356668              
  Branches    14292    14291       -1     
==========================================
+ Hits       220335   220338       +3     
+ Misses     132671   132670       -1     
+ Partials     3662     3660       -2
Impacted Files Coverage Δ
hpcgap/lib/polyconw.gi 53.79% <100%> (ø) ⬆️
lib/polyconw.gi 77.14% <100%> (ø) ⬆️
src/hpc/traverse.c 79.45% <0%> (-0.39%) ⬇️
src/funcs.c 63.78% <0%> (-0.28%) ⬇️
src/hpc/threadapi.c 30.55% <0%> (+0.09%) ⬆️
src/objset.c 39.88% <0%> (+1.12%) ⬆️
src/hpc/tls.h 67.5% <0%> (+2.5%) ⬆️

@fingolfin
Copy link
Member

For me, this warning is in so far useful as it usually tells me I need to try something a bit different, as I am leaving the "fast path".

That said, I certainly will survive if this is changed. shrug

Perhaps @hulpke has a strong opinion on this, though? (given that this also affects e.g. matrix groups)

@fingolfin fingolfin requested a review from hulpke May 28, 2017 19:44
@ChrisJefferson
Copy link
Contributor Author

My feeling is there any many, many algorithms in GAP which are "slow", we could warn whenever partition backtrack isn't performing well (or is invoked at all), when we have to verify a stabilizer chain, etc. etc.

Of course, there might be some deeper reason that the conway polynomials are different -- in which case we should at least attach some kind of help message, to tell me (and others) why we should care :)

@fingolfin
Copy link
Member

Well, as I said, I am fine with increasing the level here anyway :-)

Copy link
Contributor

@hulpke hulpke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As there is a higher level warning if the computation is really hard (e.g. for GF(1013^10)) I cannot see a need for warning about these basic calculations.

@fingolfin fingolfin merged commit 27bd4b1 into gap-system:master May 29, 2017
@olexandr-konovalov
Copy link
Member

Maybe hard cases could be indicated with InfoPerformance, introduced not so far and still rarely used.

@ChrisJefferson ChrisJefferson deleted the conway branch June 2, 2017 17:14
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.9.0 milestone Jan 20, 2018
@olexandr-konovalov olexandr-konovalov added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Jan 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: added PRs introducing changes that have since been mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants