Submitting author: @pachamaltese (Mauricio Vargas)
Repository: https://github.com/pachamaltese/gravity
Version: v0.8.2
Editor: @yochannah
Reviewers: @DiegoKoz
Author instructions
Thanks for submitting your paper to JOSS @pachamaltese. The JOSS editor (shown at the top of this issue) will work with you on this issue to find a reviewer for your submission before creating the main review issue.
@pachamaltese if you have any suggestions for potential reviewers then please mention them here in this thread. In addition, this list of people have already agreed to review for JOSS and may be suitable for this submission.
Editor instructions
The JOSS submission bot @whedon is here to help you find and assign reviewers and start the main review. To find out what @whedon can do for you type:
@whedon commands
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks.
For a list of things I can do to help you, just type:
@whedon commands
Attempting PDF compilation. Reticulating splines etc...
@whedon assign @yochannah as editor
OK, the editor is @yochannah
@pachamaltese thanks for your submission! Is there anybody you can think of who might be interested in reviewing this paper?
👋 Hey @xuanxu and @vyasr, is this a package either or both of you might be interested in reviewing?
Hi @yochannah https://github.com/yochannah. I have no clue. Sorry!
On Sat, Sep 8, 2018, 7:05 PM Yo Yehudi notifications@github.com wrote:
@pachamaltese https://github.com/pachamaltese thanks for your
submission! Is there anybody you can think of who might be interested in
reviewing this paper?—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/openjournals/joss-reviews/issues/926#issuecomment-419675857,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AJn6OdJxHdFU1GQEsOOMvkvlLe8CXg0xks5uZD8rgaJpZM4WYJD5
.
Hmm no sorry, this doesn't look like something I'd know enough to review. Looks cool though!
No worries @vyasr and thanks for responding.
👋 Hey @khaors, might this be something you's be able to review?
Still hunting for reviewers - the majority of reviewers in the JOSS reviewer list tend to focus on bioinformatics rather than physics related subjects.
@cole-brokamp, would you be interested in reviewing this?
this is not physics!! it's economics
El mar., 11 de sep. de 2018 a la(s) 16:21, Yo Yehudi (
[email protected]) escribió:
Still hunting for reviewers - the majority of reviewers in the JOSS
reviewer list
https://docs.google.com/spreadsheets/d/1PAPRJ63yq9aPC1COLjaQp8mHmEq3rZUzwUYxTulyu78/edit#gid=856801822
tend to focus on bioinformatics rather than physics related subjects.@cole-brokamp https://github.com/cole-brokamp, would you be interested
in reviewing this?—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/openjournals/joss-reviews/issues/926#issuecomment-420391265,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AJn6OTKXddNPoe1r42SbQc6SvOOgoE2uks5uaA0dgaJpZM4WYJD5
.
@cole-brokamp, would you be interested in reviewing this?
Looks interesting, but I think the domain is outside of my area of expertise. Sorry I couldn't be of more help!
Thanks @cole-brokamp for letting us know!
@pachamaltese Yikes, I'm really sorry for the mistake! Thank you for pointing my error out.
@pachamaltese I haven't had any responses from the reviewers I've emailed so far unfortunately, but I've emailed a few more so hopefully we'll have someone soon. In the meantime I noticed that a couple of the links in your reference section have what I'm guessing is a copy/paste error - showing https://doi.org twice, like this:
@yochannah thanks for letting me know !! I actually fixed that in the original repository. How can I reflect those changes here?
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@pachamaltese That looks mostly better! The first link is fixed, but the final link in the references section is still broken - it looks ok visually but returns an error when you click.
@yochannah thanks a lot !! that was an error, I fixed that here https://github.com/pachamaltese/gravity
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
Looks good now!
I think @DiegoKoz may be interested in reviewing! 🎉 I'm still on the hunt for a second reviewer - quite a few haven't responded to my emails thus far.
Hi @pachamaltese, its a pleasure!
As I told @yochannah, it's the first time I do a review, and especially of this kind, I'll try to do my best
Hi @pachamaltese, while looking for a second reviewer, let me make some comments about documentation.
It would be nice that the vignette does more than creating the dataset. It's great that you include the dataset from Head and Mayer, It would be fantastic if you make a vignette where you use it for the different functions, and which are the advantages from each way of estimating the gravity model. I don't think this is an indispensable criteria for accepting the paper, because the documentation inside the functions seems enough, but it would be a nice thing to have. At least a minimal example in the readme of the repo.
Another thing you should add to the Readme is the Community guidelines: There should be clear guidelines for third-parties wishing to contribute, report issues and seek support
thanks a lot @DiegoKoz
of course I'm expanding the vignettes
in the documentation of the functions, the regressors
param, it says "curreny", I think it is repeated in all the functions.
It's kind of problematic that the function renames the first input from the regressors
param, and always assume that is a distance. It would be better to add the parameter distance
, as you added the incomes
as a separate thing. For example, I run the following:
bvu(dependent_variable = "flow", regressors = c("flow","contig","rta"),
incomes = c("gdp_o", "gdp_d"), codes = c("iso_o", "iso_d"),
robust = TRUE, data = gravity_no_zeros)
And didn't make any warning.
In general, I'm not comfortable with the strict dependence of the ordering (inc_0, inc_d). I think it would be better to have different parameters for those things, to avoid confusion.
The function works as it is, and the documentation clarify the needed order, so just take it as a suggestion.
Also, I don't understand why you choose to return de summary of the models instead of the object? From what I see, this is general for all the functions. Returning the object you fit would allow the users to predict, make intervals, etc. And also use the modelr
and broom
packages.
The summary was to do exactly what the Stata package does, and you are right that "free translation" would be better
@pachamaltese I think adding the reviewer as a contributor would emerge a conflict of interests (right @yochannah ?) but thanks anyway!!
The example in ?nbpml()
is not done _the tidy way_ as the rest of the documentation.
Also with nbpml()
, my computer just crashed (twice) because of the amount of computation needed. This is because of MASS::glm.nb()
is computationally demanding, right? I'm using the examples in the documentation.
@DiegoKoz ok, no worries!
yeah, glm eats a lot of ram, you can check inside nbpml.R
and see the short example for CRAN (i.e. a test with a smaller dataset that takes <10 seconds)
I think it's more a matter with CPU rather than RAM.
The example inside nbpml.R runs, but gives lot's of warnings like "non-integer x = 16.451930" which lend me to this: stackoverflow thread
the negative binomial is used for counting. That may be one of the reasons for the inefficiency with the computation. I assume that you didn't come up with using this distribution but you are rather reproducing it from the literature (I'm not sure from where).
If that's the case, I would suggest a warning in the documentation and removing the warnings from the output (because it is ok to give something that isn't an integer)
Take this into account:
I change "USA" for a wrong name. The error should say something about the wrong name. Probably its filtering and ending with an empty tibble that afterward gives this error message. I should be easy to check that the names are ok, right?
I might be able to help with this. could take a few months though if you need a review.
Nice to know about this package. I have one suggestion. Nowadays, one of the recommendations to assess the effects of trade policy is to use pair fixed effects (wto book). However, in some settings, this can mean the user will need a method that works with high dimensional fixed effects (in ols or ppml, for example). Otherwise, the estimation can be memory intensive and slow. After a quick look at the repository, I didn't notice any mention to that. In Stata, Thomas Zylkin wrote a specific function for the ppml case (ppml_panel_sg). For ols estimation, it is common to use the reg2hdfe
function.
In R, the user can estimate a high dimensional linear model using the lfe package. In order to allow high dimensional fixed effects in PPML, I think it's necessary to rewrite ppml model using lfe
inside. I have a sketch here.
Again, great to know about the package. I will try it soon.
@Robinlovelace thanks for the offer! @pachamaltese has been very patient, but I think that a few months might be a little bit too long :)
@paulofelipe might you be interested in reviewing this package by any chance?
@DiegoKoz once again thanks a lot for the feedback !!
I did some changes to the functions so that you can now use broom with it, and here I added a vignette (WIP) http://pacha.hk/gravity/articles/crash-course-on-gravity-models.html that explains the models
I'll add the rest of the feedback such as warnings and more, and also to shorten the paper draft a bit.
@pachamaltese this is excellent! I think this vignette will be an incentive for people to use your package.
I'll try to read it with more detail in a few days, but for now be carefull with "$ D_{ij}$" you left a space and it didn't render as formula. Also, you should add a link to this vignette in the readme, so more people can reach it!
@DiegoKoz @pachamaltese You two are doing such a great job reviewing the package I'll kick off the formal review now. I'd still like a second reviewer but may add them in later in the process.
@whedon assign @DiegoKoz as reviewer
OK, the reviewer is @DiegoKoz
@whedon start review
OK, I've started the review over in https://github.com/openjournals/joss-reviews/issues/1038. Feel free to close this issue now!
@DiegoKoz @pachamaltese - let's head over to #1038 where there is a full review checklist 👍
Most helpful comment
Hi @pachamaltese, while looking for a second reviewer, let me make some comments about documentation.
It would be nice that the vignette does more than creating the dataset. It's great that you include the dataset from Head and Mayer, It would be fantastic if you make a vignette where you use it for the different functions, and which are the advantages from each way of estimating the gravity model. I don't think this is an indispensable criteria for accepting the paper, because the documentation inside the functions seems enough, but it would be a nice thing to have. At least a minimal example in the readme of the repo.
Another thing you should add to the Readme is the Community guidelines: There should be clear guidelines for third-parties wishing to contribute, report issues and seek support