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

Refactor custom_preprocess() for more robustness #167

Merged
merged 23 commits into from
Oct 13, 2022

Conversation

dkrako
Copy link
Collaborator

@dkrako dkrako commented Oct 11, 2022

Info about PR:

Instead of passing the complete tuple with model, x_batch, y_batch, a_batch, s_batch, .. we instead pass a dictionary with the variable names as keys.

If a key matches a variable name like x_batch, it will be overwritten.

If a key refers to a new variable, this variable will be passed instance-wise to evaluate_instance().

If we pass None instead of a dictionary in custom_preprocess, no changes or additional variables will be passed to evaluate_instance.

Minimum acceptance criteria

  • All tests pass

@codecov-commenter
Copy link

codecov-commenter commented Oct 11, 2022

Codecov Report

Merging #167 (05ca1c2) into main (a004ea9) will decrease coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #167      +/-   ##
==========================================
- Coverage   94.62%   94.51%   -0.12%     
==========================================
  Files          51       51              
  Lines        2420     2370      -50     
==========================================
- Hits         2290     2240      -50     
  Misses        130      130              
Impacted Files Coverage Δ
quantus/metrics/axiomatic/completeness.py 94.28% <ø> (ø)
quantus/metrics/axiomatic/input_invariance.py 100.00% <ø> (ø)
quantus/metrics/axiomatic/non_sensitivity.py 100.00% <ø> (ø)
quantus/metrics/complexity/complexity.py 100.00% <ø> (ø)
quantus/metrics/complexity/effective_complexity.py 100.00% <ø> (ø)
quantus/metrics/complexity/sparseness.py 100.00% <ø> (ø)
...s/metrics/faithfulness/faithfulness_correlation.py 95.91% <ø> (-0.17%) ⬇️
...ntus/metrics/faithfulness/faithfulness_estimate.py 98.00% <ø> (-0.08%) ⬇️
quantus/metrics/faithfulness/infidelity.py 96.87% <ø> (-0.10%) ⬇️
quantus/metrics/faithfulness/irof.py 98.07% <ø> (-0.08%) ⬇️
... and 23 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@annahedstroem annahedstroem left a comment

Choose a reason for hiding this comment

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

Please see the comments. Some require changes on several places, like update docstrings of Returns and Params etc. Thank you!

CONTRIBUTING.md Outdated Show resolved Hide resolved
@annahedstroem annahedstroem changed the title implement more robust way of returning in custom_preprocess() Refactor custom_preprocess() for more robustness Oct 12, 2022
@dkrako dkrako force-pushed the feature/robust-custom-preprocessing branch from 0923756 to 437395b Compare October 12, 2022 21:43
…sed in any metric. Use new and flexible custom_preprocess functionality for the metrics Consistency and Sufficiency.
@dkrako dkrako force-pushed the feature/robust-custom-preprocessing branch from 437395b to 509ca7e Compare October 12, 2022 21:44
@dkrako
Copy link
Collaborator Author

dkrako commented Oct 12, 2022

Codecov Report

Merging #167 (9bf05fe) into main (eec1eef) will decrease coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #167      +/-   ##
==========================================
- Coverage   94.62%   94.51%   -0.12%     
==========================================
  Files          51       51              
  Lines        2420     2370      -50     
==========================================
- Hits         2290     2240      -50     
  Misses        130      130              

Impacted Files Coverage Δ
quantus/metrics/axiomatic/completeness.py 94.28% <ø> (ø)
quantus/metrics/axiomatic/input_invariance.py 100.00% <ø> (ø)
quantus/metrics/axiomatic/non_sensitivity.py 100.00% <ø> (ø)
quantus/metrics/complexity/complexity.py 100.00% <ø> (ø)
quantus/metrics/complexity/effective_complexity.py 100.00% <ø> (ø)
quantus/metrics/complexity/sparseness.py 100.00% <ø> (ø)
...s/metrics/faithfulness/faithfulness_correlation.py 95.91% <ø> (-0.17%) arrow_down
...ntus/metrics/faithfulness/faithfulness_estimate.py 98.00% <ø> (-0.08%) arrow_down
quantus/metrics/faithfulness/infidelity.py 96.87% <ø> (-0.10%) arrow_down
quantus/metrics/faithfulness/irof.py 98.07% <ø> (-0.08%) arrow_down
... and 23 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Coverage seems to have only decreased because this PR has a lot of deletions, so the absolute number of hits has decreased.
The absolute number of misses has not increased though, just the overall percentage of misses in relation to hits.

Copy link
Member

@annahedstroem annahedstroem left a comment

Choose a reason for hiding this comment

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

Great, let's merge!

@annahedstroem annahedstroem merged commit a2d574c into main Oct 13, 2022
@aaarrti aaarrti deleted the feature/robust-custom-preprocessing branch March 18, 2023 17:46
aaarrti pushed a commit that referenced this pull request Apr 9, 2023
…feature/robust-custom-preprocessing

Refactor custom_preprocess() for more robustness
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.

3 participants