-
Notifications
You must be signed in to change notification settings - Fork 55
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
Data types coverage & bug corrections #141
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AntoninPoche
force-pushed
the
antonin/bugs_correction
branch
2 times, most recently
from
October 19, 2023 15:42
6d17dcd
to
47397a2
Compare
This was
linked to
issues
Oct 19, 2023
AntoninPoche
force-pushed
the
antonin/bugs_correction
branch
from
October 19, 2023 16:17
47397a2
to
8c4725c
Compare
AntoninPoche
requested review from
lucashervier,
paulnovello,
Agustin-Picard,
fel-thomas and
dv-ai
October 20, 2023 08:12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work Antonin! 🔥 I just left some minor comments ;)
AntoninPoche
force-pushed
the
antonin/bugs_correction
branch
from
November 7, 2023 10:12
8c4725c
to
0dcc91c
Compare
Agustin-Picard
approved these changes
Nov 7, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work, LGTM!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
1. Data type coverage
The first part of this pull request concerns Xplique data type coverage extension. It first modifies some methods to extend the coverage, then tests the coverage and shows the application in a tutorial.
1.1 Non-square images
SobolAttributionMethod
andHsicAttributionImage
did not support non-square images due to the use ofcv2.resize()
which permutes the h and w dimensions. It was thus changed totf.image.resize()
in b77c5f3 and tested in 8b8465f.1.2 Time series & Tabular data
Xplique was initially designed for images but it also supports attribution methods for tabular data and now time series data.
Xplique conciders data with:
New attribution time series tutorial
To show how to use Xplique on time series a new tutorial was designed: Attributions: Time Series and Regression, the link was added to the documentation in 6855221.
Time series attribution plot
Furthermore, for this tutorial, time series attribution plots were updated in 397c681. The new name is
xplique.plots.plot_timeseries_attributions
with an API very similar toxplique.plots.plot_attributions
. Here is an example from the tutorial on temperature forecasting for the next 24 hours based on weather data from the last 48 hours.Update some methods
Rise
method is now applicable to tabular and time series thanks to f1c7b17, it was then tested in 8b8465f. The main modifications were for the design of the perturbation masks where a switch case on the data types was made.Then,
Lime
, andKernelshap
did not support time series natively, it was modified in 5d09b46 and tested in 8b8465f. This could be summarized as adding a defaultmap_to_interpret_space
for time seriesRelative documentation
To clarify which method supports tabular data and time series, the documentation was updated and the table of attribution methods was simplified in c131b7d:
TF : Tensorflow compatible
C : Classification | R : Regression |
OD : Object Detection | SS : Semantic Segmentation (SS)
* : See the Callable documentation
** : See the Xplique for PyTorch documentation, and the PyTorch models: Getting started notebook.
✔️ : Supported by Xplique | ❌ : Not applicable | 🔵 : Work in Progress
Test metrics
To complete time series integration in Xplique, a test to ensure metrics support them was added in fa356cc. In fact,
MuFidelity
was modified to support time series and tabular data in ec0fb20.1.3 Image explanation shape harmonization
For image explanation, depending on the method, the explanation shape could be either$(n, h, w)$ , $(n, h, w, 1)$ , or $(n, h, w, 3)$ . It was decided to harmonize it to $(n, h, w, 1)$ . This was modified in 0ae74de, tested in 8b8465f, and documented in 7a95dc7. Furthermore, unit tests needed to be modified as they hard-coded the expected output shape. Thus they were modified in 1f281c7.
Reducer for gradient-based methods
For images, most gradient-based provide a value for each channel, however, for consistency, it was decided that for images, explanations will have the shape$(n, h, w, 1)$ . Therefore, gradient-based methods need to reduce the channel dimension of their image explanations and the
reducer
parameter chooses how to do it among {"mean"
,"min"
,"max"
,"sum"
,None
}. In the caseNone
is given, the channel dimension is not reduced. The default value is"mean"
for methods exceptSaliency
which is"max"
to comply with the paper andGradCAM
andGradCAMPP
which are not concerned.2. Bugs correction
The second part of this pull request is to solve pending issues.
2.1 Memories problem
Indeed, among the reported issues several concerned memory management.
SmoothGrad, VarGrad, and SquareGrad issue #137
SmoothGrad
,VarGrad
, andSquareGrad
created tensors of shape(nb_samples, *inputs.shape[1:])
and ignored thebatch_size
parameter. Now all methods inherit fromGradientStatistic
and use an implement method to initialize, update, and get an online statistic, either the mean, the mean of squares, or the variance depending on the method. Computing those statistics online allow the method to make inference batch by batch and takebatch_size
into account.Furthermore, when
batch_size
is greater than several timesnb_samples
, several inputs are treated in the same batch. Thus allowing a greater adaptation to the memory capacity. This was implemented in 7076e9c and 4e54fdf, then tested in df4f819.MuFidelity issue #137
The metric MuFidelity had the same problem as the three previous methods, creating a tensor too large for the memory when passed to the model. It was corrected in the same way in ec0fb20 and tested in 5723855.
HsicAttributionMethod
This method had a different memory problem the
batch_size
for the model was used correctly, however, when computing the estimator a tensor of sizegrid_size**2 * nb_design**2
was created. However, for big images and/or small objects in images, thegrid_size
needs to be increased, furthermore, for the estimator to converge,nb_design
should also be increased accordingly. Which creates out-of-memory errors.Thus an
estimator_batch_size
(different from the initialbatch_size
) was introduced in 31a9674 to batch over thegrid_size**2
dimension, and tested in 3471882. The default value isNone
, thus conserving the default behavior of the method, but when an out-of-memory occurs, setting anestimator_batch_size
smaller thangrid_size**2
will reduce the memory cost of the method.2.2 Solve issues
Metrics input types issues #102 and #128
Errors were reported when using other inputs than
np.ndarray
, the problem was thatself.inputs
initialized and sanitized inBaseAttributionMetric.__init__()
were overwritten in the__init__()
other the different metrics. It was corrected in a3a76bf by making sureinputs
used go throughnumpy_sanitize
, then it was tested in daba2b4.Feature visualization latent dtype issue #131
In issue #131, @RuoyuChen10 reported a bug in the feature visualization module. A conflict in dtype between the model internal dtype and Xplique dtype. We made sure that the dtype used for the conflicting computation was the model's internal dtype. It was implemented in c3ff31b.
2.3 Other corrections
Naturally, other problems were reported to us outside of issues or discovered by the team, we also addressed these.
Some refactorization
Lime
:Pylint was not happy with all the if-else branches to determine default values for
map_to_interpret_space
andref_value
. Those cases were moved fromexplain()
to_set_shape_dependant_parameters()
, the latter being called byexplain()
. This was done to remove the_compute()
method and place the main part of the code in theexplain()
method for consistency between methods. This was done in 1e3c993.Typo and small fixes
In
HsicAttributionMethod
andSobolAttributionMethod
there was a difference between the documentation of theperturbation_function
and the actual code. It was corrected in abd7975.Other typos and small mistakes were corrected in fb58c3a and 59d441a.
For Craft, there were some remaining prints, but they may be useful, thus Craft's methods with print now take a
verbose
parameter. Implemented in d696f93.In Craft tests, a PyTorch tensor conversion bug was resolved in d6e1465.
Pylint raised
no-member
errors for some of PyTorch's functions, this comes from the fact that Pylint does not recognize dynamically set members. Therefore,no-member
was added to the list of disabled Pylint errors in 0dcc91c.