-
Notifications
You must be signed in to change notification settings - Fork 133
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
QuadFormWithIsom
Patch 2: towards better time in CI
#2825
Conversation
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.
Looks plausible to me, thanks!
Any noticeable improvements for compilation timings? |
Still waiting for ci. We had to wait for #2826 to be merged |
Codecov Report
@@ Coverage Diff @@
## master #2825 +/- ##
==========================================
- Coverage 80.63% 80.47% -0.16%
==========================================
Files 456 456
Lines 64646 64945 +299
==========================================
+ Hits 52127 52266 +139
- Misses 12519 12679 +160
|
CI doesn't seem to give reliable timings (aka there were differences in compilation time when no changes in this code were made, so I do not see a good baseline to compare against) |
@thofma we have at least better test timings once everything is compiled |
You can't do timings using CI. First off, different machines run CI jobs, they can vary by their performance by a factor 2. Then on the same machine, timings can vary depending on how many VMs are active on that machine at a time, and how active each is. And then there is randomization, which should maybe not affect compilation time, but certainly can affect runtime greatly. The only good way to compare timings before / after is to take an example input and compute it with the old and with the new code, on the same machine, which should not be busy (if it is a laptop, also plug it in, etc. -- but better test on a server, not a laptop or PC, they have far more flucation). Ideally also use And to avoid random fluctuations, call
|
We fix some type instability, remove unnecessary checks and improve some functions, with @lgoettgens
orbit_representatives_and_stabilizers
type stableAutomorphismGroup{<:Union{TorQuadModule, GrpAbFinGen}}
base_ring
for matrix groups type stableQuadFormWithIsom
QuadFormWithIsom
Related to #2599
@fingolfin could you check if the GAP-related changes are ok ?