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

ENHANCE: Avoid memory issues in solvable conjugacy classes routine. #3078

Merged
merged 4 commits into from
Dec 19, 2018

Conversation

hulpke
Copy link
Contributor

@hulpke hulpke commented Dec 3, 2018

In cases of many classes being created by acting on a large vector space, there can be significant memory churning by recreating dictionaries and orbits. Avoid doing so.

Changing dictionary limits also pointed out some shortcomings with small matrix groups over large fields.

Finally, try to reduce the size of the vector space, if possible.

@hulpke hulpke added topic: performance bugs or enhancements related to performance (improvements or regressions) do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) labels Dec 3, 2018
@codecov
Copy link

codecov bot commented Dec 3, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@ddf633d). Click here to learn what that means.
The diff coverage is 91.57%.

@@            Coverage Diff            @@
##             master    #3078   +/-   ##
=========================================
  Coverage          ?   83.58%           
=========================================
  Files             ?      686           
  Lines             ?   336845           
  Branches          ?        0           
=========================================
  Hits              ?   281536           
  Misses            ?    55309           
  Partials          ?        0
Impacted Files Coverage Δ
lib/grpmat.gi 71.2% <100%> (ø)
lib/dict.gi 77.47% <100%> (ø)
lib/pcgs.gd 100% <100%> (ø)
lib/oprtpcgs.gi 90.24% <100%> (ø)
lib/claspcgs.gi 47.1% <76.19%> (ø)
lib/pcgs.gi 81.97% <94.54%> (ø)

@hulpke hulpke changed the title ENHANCE: Avoid memory churning in solvable conjugacy classes routine. ENHANCE: Avoid memory issues in solvable conjugacy classes routine. Dec 3, 2018
lib/dict.gi Outdated Show resolved Hide resolved
@hulpke hulpke force-pushed the additions branch 2 times, most recently from 5c1b530 to 88afc74 Compare December 11, 2018 16:54
@hulpke hulpke removed the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Dec 11, 2018
lib/claspcgs.gi Outdated
@@ -325,6 +325,7 @@ local classes, # classes to be constructed, the result
depthlev, # depth at which N starts
one,zero,
vec,
dict,
Copy link
Member

Choose a reason for hiding this comment

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

Minor nitpick: tabs vs. spaces...

Reduce assertion level in test, as the many tests otherwise make it unusable
Use option to enforce longer blist, as its used anyhow.
This avoids enumerating a large vector space, but is not always possible.
@fingolfin fingolfin merged commit b70dcc9 into gap-system:master Dec 19, 2018
@wilfwilson wilfwilson added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Apr 15, 2019
@DominikBernhardt DominikBernhardt added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 22, 2019
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.11.0 milestone Aug 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes topic: performance bugs or enhancements related to performance (improvements or regressions)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants