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.

Sign up for GitHub

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

Jump to bottom

Demodulation cosmetics #539

Merged
merged 4 commits into from
Mar 22, 2024
Merged

Demodulation cosmetics #539

merged 4 commits into from
Mar 22, 2024

Conversation

quickbeam123
Copy link
Collaborator

  1. getDemodulationLHSIterator does not need to be given the options object (we already know at the sites where we use it, whether we are doing FWD or BWD)
  2. call SortHelper::getEqualityArgumentSort exactly once in withEqualitySort (and not potentially twice); but don't use on empty iterators

These are almost trivial changes. Just something I noticed before the breakfast...

quickbeam123 and others added 2 commits March 20, 2024 08:05
@quickbeam123
getDemodulationLHSIterator does not need to read the Options
4c85494
in EqHelper, make the helper function withEqualitySort compute getEqu…
ef1f29a 
…alityArgumentSort exactly once; so don't use on empty iters; should help for iters with more than one item
MichaelRawson
MichaelRawson approved these changes Mar 20, 2024
View reviewed changes
Copy link
Contributor

@MichaelRawson MichaelRawson left a comment

Choose a reason for hiding this comment

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

Agreed, cosmetically better and we should generally reduce use of env.

@mezpusz mezpusz force-pushed the martin-demodulation-cosmetics branch from baa9374 to 4ff35ae Compare March 21, 2024 11:14
@quickbeam123
Copy link
Collaborator Author

Strictly better for discount10:

Sort by SAT
1006 ['problemsSTD_master7392_dis10_i10000.pkl']
1006 ['problemsSTD_mdc7396_dis10_i10000.pkl']

Sort by UNS
7763 ['problemsSTD_master7392_dis10_i10000.pkl']
7766 ['problemsSTD_mdc7396_dis10_i10000.pkl']

Total 1006 / 7766

Greedy cover (UNS):
# problemsSTD_mdc7396_dis10_i10000.pkl contributes 7766 total 7766
Total 7766

joe-hauns
joe-hauns reviewed Mar 22, 2024
View reviewed changes
Copy link
Contributor

@joe-hauns joe-hauns left a comment

Choose a reason for hiding this comment

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

Looks all good to me! Loving the code deduplication using DemodulationHelper!

@MichaelRawson MichaelRawson merged commit 23d7e9c into master Mar 22, 2024
1 check passed
@MichaelRawson MichaelRawson deleted the martin-demodulation-cosmetics branch March 22, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelRawson MichaelRawson MichaelRawson approved these changes

@joe-hauns joe-hauns joe-hauns approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@quickbeam123 @MichaelRawson @joe-hauns @mezpusz