Skip to content

Excluding merged inits#25

Merged
okofish merged 3 commits intomasterfrom
feature/exclude-merged-states
Mar 12, 2021
Merged

Excluding merged inits#25
okofish merged 3 commits intomasterfrom
feature/exclude-merged-states

Conversation

@okofish
Copy link
Copy Markdown
Collaborator

@okofish okofish commented Mar 4, 2021

Changes

  • Add an EvaluationParameters option to include/exclude inits that merged with another init (set to exclude by default)

Comments

  • It turns out that in large scale computations ~99.9% of inits end up with this conclusion reason (which is good; it means the shared trie is very effective). These are of limited scientific interest, and discarding them massively reduces the size of output files.
  • We may eventually want a generic mechanism for choosing the conclusion reasons to keep/discard. This could then be exposed on the command line.

This change is Reviewable

@okofish okofish requested a review from maxitg March 4, 2021 01:40
Copy link
Copy Markdown
Owner

@maxitg maxitg left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @okofish)


libPostTagSystem/PostTagSearcher.hpp, line 55 at r1 (raw file):

    Checkpoints checkpoints = {};
    bool includeUnevaluatedStates = false;
    bool includeMergedStates = false;

We need to either fix the failing test or make includeMergedStates = true by default.

Copy link
Copy Markdown
Collaborator Author

@okofish okofish left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 3 files reviewed, all discussions resolved (waiting on @okofish)


libPostTagSystem/PostTagSearcher.hpp, line 55 at r1 (raw file):

Previously, maxitg (Max Piskunov) wrote…

We need to either fix the failing test or make includeMergedStates = true by default.

I set includeMergedStates = true for this test.

@okofish okofish merged commit 71449c0 into master Mar 12, 2021
@okofish okofish deleted the feature/exclude-merged-states branch March 12, 2021 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants