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

add counterfactual code #98

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
Open

Conversation

shahashka
Copy link

No description provided.

@rajeeja
Copy link
Contributor

rajeeja commented Sep 30, 2021

@jmohdyusof @j-woz please comment for changes on this.

@rajeeja
Copy link
Contributor

rajeeja commented Sep 30, 2021

maybe we can get rid of the notebooks.

@jmohdyusof
Copy link
Contributor

I am not sure why this is structured like this, with an entirely separate subdirectory within NT3? Either make it a separate benchmark (like e.g. Uno_UQ, UnoMT) or incorporate it into the NT3 directly (it seems like some files are duplicated from the original NT3?)

@shahashka
Copy link
Author

Yes, this was just to get all the code up for everyone to view. Things will have to change. A lot of the reason for the duplicated files was because all my files are in pickle format, whereas the original files read csv's. I can add a pickle reader to the existing code and restructure things around.

@jmohdyusof
Copy link
Contributor

There is a lot of cleanup needed in that repo anyway, like consolidating all the noise stuff into a single add_noise call that ingests all the gParams, so that all the logic is at the 'common' level. I have that in another branch and will be doing some changes during hte hackathon this week. If you can add the pickle reader in a nice modular way that would be good.

@shahashka
Copy link
Author

I'm thinking of adding some gParameters: a flag for saving to pickle for the baseline, and a string for input cf noise file. Would this be reasonable? To do this, I need to edit the parse_utils.py file I think

@jmohdyusof
Copy link
Contributor

jmohdyusof commented Oct 4, 2021

Yeah, I would start a noise_conf list, and I can add in the new params in there later (noise_type, noise_features, noise_samples etc). We want to enforce that all the names start with noise_ so that they all get grouped together and are easy to find.

@shahashka
Copy link
Author

shahashka commented Oct 4, 2021

ok sounds good. i will also run all the cf stuff end to end on a smaller data size to make sure its all working and remove some explicit file paths to my directory

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