search

sahirbhatnagar / cbpaper

Source code for Casebase paper
Other
2 stars 1 forks source link

Red comments fixed, switched to noninteraction models #17

Closed Jesse-Islam closed 4 years ago

Jesse-Islam commented 4 years ago

need to use IPA and fix lollipop plot.

turgeonmaxime commented 4 years ago

Thanks @Jesse-Islam for the pull request. But I'm a bit confused by your comment above:

need to use IPA and fix lollipop plot.

Is this what the PR does, or what you wish it did? In other words, can I merge this into the master branch, or do you plan to add more commits?

Jesse-Islam commented 4 years ago

Yes! its more an update of what was done and what's still left

an issue though, its not building on travis (my personal fork) and im working on that.

turgeonmaxime commented 4 years ago

@Jesse-Islam can you clarify which "personal fork" does not build on Travis? Is it for this paper (cbpaper) or for the package (casebase)?

Jesse-Islam commented 4 years ago

The problem was with cbpaper, but so long as it's working on your end that's great! For casebase, I'm still in the testing phase of IPA, as there is a disagreement between my repurposed code and the method in riskRegression. This will become a proper function when I pull request it.

On Thu., Feb. 13, 2020, 9:16 a.m. Maxime Turgeon, notifications@github.com wrote:

@Jesse-Islam https://github.com/Jesse-Islam can you clarify which "personal fork" does not build on Travis? Is it for this paper (cbpaper) or for the package (casebase)?

-

The cbpaper repo looks fine to me, every build was OK: https://hub.docker.com/r/sahirbhatnagar/cbpaper/builds

I had a quick look at your fork of casebase, and the file IPA.R is problematic. In particular, on the first line you're importing casebase, which means that every time you run library(casebase), it sources the file IPA.R, which runs library(casebase), which sources IPA.R, etc. So you're stuck in an infinite loop.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sahirbhatnagar/cbpaper/pull/17?email_source=notifications&email_token=AGOHSPDLHJ34BWRSA756IPTRCVI45A5CNFSM4KRFLYO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELVDT6Q#issuecomment-585775610, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGOHSPGPAGUAYG7TRXOVNGLRCVI45ANCNFSM4KRFLYOQ .

turgeonmaxime commented 4 years ago

As discussed this morning, I'll be merging this. The remaining changes to the regularized regression case study will be part of a future pull request.