Skip to content

Adding GeCo to Carla#154

Open
GibbsG wants to merge 6 commits intocarla-recourse:mainfrom
GibbsG:main
Open

Adding GeCo to Carla#154
GibbsG wants to merge 6 commits intocarla-recourse:mainfrom
GibbsG:main

Conversation

@GibbsG
Copy link

@GibbsG GibbsG commented Apr 26, 2022

Hi,

This PR is for adding GeCo, a counterfactual model, to Carla Benchmark. I also added the test in test/test_cfmodel.py and also updated the experiments to include GeCo.

Please have a look!

Thanks!
Gibbs

@JohanvandenHeuvel
Copy link
Collaborator

JohanvandenHeuvel commented Apr 26, 2022

Hi, thanks for submitting your PR.

Some comments on first glance:

  • Could you add ".DS_Store" to the .gitignore, and remove those files from the PR?
  • Did you follow this: https://carla-counterfactual-and-recourse-library.readthedocs.io/en/latest/installation.html#contributing? It seems there are some small problems with the formatting of the code. If there is anything unclear there please let us know, then we can add improvements.
  • Also I think Julia needs to be added to the requirements. Could you tell which version you use? (and if there are also other important packages, with their version) I'll try to fix that for you then, because there are multiple files with requirements so that's always a bit confusing.

Copy link
Collaborator

@indyfree indyfree left a comment

Choose a reason for hiding this comment

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

@GibbsG thanks for submitting your first PR! A great addition to also have some Julia based models included. See few comments below

@GibbsG
Copy link
Author

GibbsG commented May 23, 2022

Hi, thanks for submitting your PR.

Some comments on first glance:

  • Could you add ".DS_Store" to the .gitignore, and remove those files from the PR?
  • Did you follow this: https://carla-counterfactual-and-recourse-library.readthedocs.io/en/latest/installation.html#contributing? It seems there are some small problems with the formatting of the code. If there is anything unclear there please let us know, then we can add improvements.
  • Also I think Julia needs to be added to the requirements. Could you tell which version you use? (and if there are also other important packages, with their version) I'll try to fix that for you then, because there are multiple files with requirements so that's always a bit confusing.

Hi @JohanvandenHeuvel thanks for reviewing it. I think most of them are addressed. The packages for GeCo itself are located at this file: carla/recourse_methods/catalog/geco/library/GeCo.jl/Manifest.toml. To run Julia in Python, we need one additional package from here https://pyjulia.readthedocs.io/en/latest/installation.html.

@GibbsG
Copy link
Author

GibbsG commented May 23, 2022

@GibbsG thanks for submitting your first PR! A great addition to also have some Julia based models included. See few comments below

Hi @indyfree, I really appreciate your comments. Please have a review again! Thanks.

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.

4 participants