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

Importance of hyperparameters #737

Merged
merged 22 commits into from
Apr 9, 2024
Merged

Importance of hyperparameters #737

merged 22 commits into from
Apr 9, 2024

Conversation

elboyran
Copy link
Contributor

Solves issue #697.

@elboyran
Update README.md
8b15b63 
Stating the importance to hyperparameters. Link to Willem's thesis and the Tutorials README, where the actual values are given.
@elboyran elboyran self-assigned this Mar 27, 2024
@elboyran elboyran linked an issue Mar 27, 2024 that may be closed by this pull request
Add notes about XAI hyperparameters importance in all tutorials ad/or tutorials/main README #697
Closed
@elboyran
Update README.md
f2aeaaa 
Added the default hyperparameter values from Willem's thesis (table 5.2) for the 3 explainers. Do not know how to make it in a single compact table (couldn't find how to do it in Github MD).

Verifying he values from the tutorials and adding those for tabular and TS is next TODO.
@elboyran
Update README.md
bfcaffb 
Fixing tutorial links from logos
@elboyran
Update README.md
7bf713c 
Fixing broken link to FRB tutorial.
@elboyran
Update README.md
4b167b7 
Default and tutorials hyprparameter values for RISE.
@loostrum is the feature resolution 1 (tutorial divides one of the single dimension in 4 with the note that "feature size is roughly 4 timesteps")?
@elboyran
Update README.md
9aaa630 
Link to the exact section in the Tutorials' README about hyperparamters
@elboyran
Update README.md
ba1b8af 
Expanding the FRB abbreviation.
@elboyran
Update README.md
c69b5d6 
Emphasizing the different elements important for tutorials- themselves, data, models and hyperparameters.
@elboyran
Update README.md
3c0a162 
Move the data as first table and add the task to be solved by the model to be explained!
@elboyran
Update README.md
25723c7 
LIME hyperparameters
@elboyran
Update README.md
864abe9 
KernalSHAP hyperparameters
@elboyran elboyran mentioned this pull request Mar 27, 2024
Closed
@elboyran elboyran marked this pull request as ready for review March 27, 2024 15:26
@elboyran
Update README.md
6fe2beb 
Fixing broken tutorial link.
@elboyran
Update README.md
d6f5870 
Trying to fix (broken?) TF link
@elboyran
Update README.md
1f9c204 
Official link to Tensorflow doesn't pass the checks (?), using the GitHub page instead.
@elboyran elboyran changed the title Iportance of hyperparameters Importance of hyperparameters Mar 27, 2024
@elboyran
Update README.md
52339fb 
Correct feature resolution for the FRB case.
@SarahAlidoost SarahAlidoost self-requested a review April 3, 2024 09:57
@elboyran elboyran removed the request for review from cwmeijer April 3, 2024 13:32
@elboyran elboyran mentioned this pull request Apr 5, 2024
Closed
4 tasks
cwmeijer
cwmeijer requested changes Apr 8, 2024
View reviewed changes
Copy link
Contributor

@cwmeijer cwmeijer left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Makes the whole hyperparameter issues much clearer than before!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
tutorials/README.md Show resolved Hide resolved
tutorials/README.md Outdated Show resolved Hide resolved
tutorials/README.md Show resolved Hide resolved
tutorials/README.md Outdated Show resolved Hide resolved
tutorials/README.md Outdated Show resolved Hide resolved
tutorials/README.md Show resolved Hide resolved
elboyran and others added 4 commits April 8, 2024 10:44
@elboyran @cwmeijer
Update README.md
a224242 
Co-authored-by: Christiaan Meijer <c.meijer@esciencecenter.nl>
@elboyran @cwmeijer
Update README.md
854fa2c 
Co-authored-by: Christiaan Meijer <c.meijer@esciencecenter.nl>
@elboyran @cwmeijer
Update tutorials/README.md
080bb1e 
Co-authored-by: Christiaan Meijer <c.meijer@esciencecenter.nl>
@elboyran @cwmeijer
Update tutorials/README.md
508f60d 
Co-authored-by: Christiaan Meijer <c.meijer@esciencecenter.nl>
elboyran and others added 2 commits April 8, 2024 10:47
@elboyran @cwmeijer
Update tutorials/README.md
636567b 
Co-authored-by: Christiaan Meijer <c.meijer@esciencecenter.nl>
@elboyran @cwmeijer
Update tutorials/README.md
7fe88b7 
Co-authored-by: Christiaan Meijer <c.meijer@esciencecenter.nl>
@elboyran elboyran mentioned this pull request Apr 8, 2024
Closed
5 tasks
@elboyran
Copy link
Contributor Author

elboyran commented Apr 8, 2024 •
edited
Loading

@cwmeijer unfortunately the check still doesn't like the TF link (super weird!). Shall we use the link I've suggested previously?

@elboyran elboyran requested a review from cwmeijer April 8, 2024 09:00
@elboyran
Copy link
Contributor Author

elboyran commented Apr 8, 2024 •
edited
Loading

Ah, the notebooks fail now, for some reason the output of kernalshap explain expects 1, not 2. But in my branch code still expect 2, not 1 as in the update_notebooks branch. So weird.

@SarahAlidoost
Copy link
Contributor

Ah, the notebooks fail now, for some reason the output of kernalshap explain expects 1, not 2. But in my branch code still expect 2, not 1 as in the update_notebooks branch. So weird.

The GitHub Action is checking out the merge commit of your pull request. This means that the workflow is operating on the code as it would be if the pull request were merged into the main branch. This is the standard behavior for the pull_request event in GitHub Actions. The notebooks are broken in the main branch, that's why we see the failed action here too.

@SarahAlidoost
Copy link
Contributor

@cwmeijer if you are happy with the changes, can you please approve this pull request? Then we can merge it. thanks.

cwmeijer
cwmeijer approved these changes Apr 9, 2024
View reviewed changes
Copy link
Contributor

@cwmeijer cwmeijer left a comment

Choose a reason for hiding this comment

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

🏆🍹

@SarahAlidoost SarahAlidoost merged commit 4dd0bc4 into main Apr 9, 2024
14 of 17 checks passed
@SarahAlidoost SarahAlidoost deleted the 697-hyperparameters branch April 9, 2024 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cwmeijer cwmeijer cwmeijer approved these changes

Assignees

@elboyran elboyran

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add notes about XAI hyperparameters importance in all tutorials ad/or tutorials/main README
4 participants
@elboyran @SarahAlidoost @cwmeijer @loostrum