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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

If only one filter selected use it for evaluations #582

Merged
merged 3 commits into from
Dec 10, 2023

Conversation

andrews05
Copy link
Collaborator

@andrews05 andrews05 commented Nov 20, 2023

The filters used during reduction evaluations are normally set to None & Bigrams, regardless of any options. This PR makes a slight change so that if only one filter is specified in the options, this filter will be used for reduction evaluations too.

This resolves an odd situation affecting lower levels (when --fast is enabled) where you may try to force the filter to a specific value but it actually ends up different because a reduction evaluation was smaller. It's particularly helpful if you're wanting it to be as fast as possible by using -o0 -f0 which will now exclusively use None instead of trying the slower Bigrams as well.

As another use, you could try to brute force oxipng by iterating each filter separately, though this may not actually achieve anything 馃槀

[edit] I also pulled the options out into a separate file, though this wasn't relevant to the filter change.

@AlexTMjugador
Copy link
Collaborator

I like the overall idea of this PR, but it makes me reflect on how we should handle the case of selecting exactly two filters. What are your thoughts on just cloning the input filters for evaluation if there is either one or two?

@andrews05
Copy link
Collaborator Author

Yeah, I thought about that for a bit, and then what about cases where there are more than two input filters but still excluding None or Bigrams, and it all got a bit complicated. I ended up doing it this way for two reasons:

  • The case of only one filter is more likely/more important than other non-default filter selections.
  • The standard 0,7 is actually still a good choice for some combos such 0,9.

@AlexTMjugador
Copy link
Collaborator

Sounds fair enough to me!

The standard 0,7 is actually still a good choice for some combos such 0,9.

I guess this observation could be extended to mean that the standard 0,7 is also a good proxy for combos of most other filter combinations? How big is the some combos set here?

@andrews05
Copy link
Collaborator Author

Generally speaking I'd say 7 is a good proxy for any heuristic filter (5-9). Hypothetically, it may not be optimal if you only chose non-heuristic filters (0-4). Similarly, 0,7 may not be optimal if your combo excludes 0. I haven't exactly performed any analysis to test these though and I imagine any impact would be pretty small.
These situations would also be quite unexpected with no obvious use-case, so I felt it wasn't worth the complexity required to account for them.

Copy link
Collaborator

@AlexTMjugador AlexTMjugador left a comment

Choose a reason for hiding this comment

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

Okay, I'm convinced, let's move forward with this. Thanks for the PR!

@AlexTMjugador AlexTMjugador merged commit f6c2440 into shssoichiro:master Dec 10, 2023
10 checks passed
@andrews05 andrews05 deleted the feature/eval-filters branch December 24, 2023 06:41
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.

None yet

2 participants