openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
725 stars 38 forks source link

[REVIEW]: MODE.behave: A Python Package for Discrete Choice Modeling #5265

Closed editorialbot closed 1 year ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@julianreul<!--end-author-handle-- (Julian Reul) Repository: https://github.com/julianreul/mode_behave Branch with paper.md (empty if default branch): Version: v1.0.13 Editor: !--editor-->@sbenthall<!--end-editor-- Reviewers: @abhiramm7, @fabmio Archive: 10.5281/zenodo.8225871

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/baa234887efca533771d9d78ccf837d3"><img src="https://joss.theoj.org/papers/baa234887efca533771d9d78ccf837d3/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/baa234887efca533771d9d78ccf837d3/status.svg)](https://joss.theoj.org/papers/baa234887efca533771d9d78ccf837d3)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@l-kotzur & @abhiramm7, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @sbenthall know.

Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest

Checklists

📝 Checklist for @fabmio

📝 Checklist for @abhiramm7

editorialbot commented 1 year ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 1 year ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.06 s (264.8 files/s, 91986.2 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          10            658           1105           2818
reStructuredText                 1             67              0            227
Markdown                         1             26              0            137
TeX                              1             14              0            120
YAML                             2              1              4             33
-------------------------------------------------------------------------------
SUM:                            15            766           1109           3335
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago

Wordcount for paper.md is 1172

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

sbenthall commented 1 year ago

👋🏼 @l-kotzur, @abhiramm7, @julianreul this is the review thread for the paper. All of our communications will happen here from now on.

As a reviewer, the first step is to create a checklist for your review by entering

@editorialbot generate my checklist

as the top of a new comment in this thread.

These checklists contain the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. The first comment in this thread also contains links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention https://github.com/openjournals/joss-reviews/issues/5265 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 2-4 weeks. Please let me know if any of you require some more time. We can also use EditorialBot (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@sbenthall) if you have any questions/concerns.

editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/S1755-5345(13)70022-8 is OK
- 10.1016/j.jocm.2016.07.004 is OK
- 10.1016/j.jocm.2021.100339 is OK
- 10.1016/j.jocm.2021.100284 is OK

MISSING DOIs

- 10.1016/0191-2607(88)90064-7 may be a valid DOI for title: Discrete Choice Analysis: Theory and Application to Travel Demand
- 10.1016/0191-2607(88)90041-6 may be a valid DOI for title: Qualitative Choice Analysis: Theory, Econometrics, and an Application to Automobile Demand
- 10.1002/1099-1255(200009/10)15:5<447::aid-jae570>3.0.co;2-1 may be a valid DOI for title: Mixed MNL models for discrete response
- 10.2139/ssrn.4230060 may be a valid DOI for title: Modeling Behavioral Change in Transport Futures

INVALID DOIs

- None
l-kotzur commented 1 year ago

Review checklist for @l-kotzur

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

l-kotzur commented 1 year ago

@julianreul Shall https://github.com/julianreul/mode_behave or https://github.com/FZJ-IEK3-VSA/mode_behave be the place of the repository?

l-kotzur commented 1 year ago

@julianreul

it is the first to provide functionality for the estimation of mixed logit models with nonparametric distribution

What is the advantage or benefit of a nonparametric distribution to a parametric distributions of the other packages for the application cases?

l-kotzur commented 1 year ago

@julianreul Could you add the code on how to reproduce the plots in the imgs-folder?

Can you use English names instead of German names for the parameters in example_estimation.py?

Can you either add the data for the Japanese market or drop the plots? They are not reproducible.

Can you remove dist folder from the repo? It makes the package unnecessarily large.

l-kotzur commented 1 year ago

@julianreul Can you integrate automated runs of your tests scripts, e.g., to Github workflow?

l-kotzur commented 1 year ago

@julianreul Could you add contribution guidelines to the repo?

l-kotzur commented 1 year ago

@julianreul Could you modify the core module (or extract) such that the input data and model parameters can be directly provided either as numpy arrays or pandas dataframes? In its current application it cumbersome to integrate the package in an workflow since that data cannot be directly be given to the model.

Can you explain the .picke files in ModelParam? Are they necessary for the flow? If so, could you use a human-readable datatype such as .json or .csv? In their current form it is not transparent how they integrate to the model.

julianreul commented 1 year ago

Dear @l-kotzur , thanks for your feedback. - I managed to work on most of your comments. Changes are commited to the "review"-branch of the repo. Further explanations in response to your comments are given below:

Shall https://github.com/julianreul/mode_behave or https://github.com/FZJ-IEK3-VSA/mode_behave be the place of the repository? (DONE)

The place of the reference-repository shall be https://github.com/julianreul/mode_behave, as this was indicated as the reference-repo during the submission process.

What is the advantage or benefit of a nonparametric distribution to a parametric distributions of the other packages for the application cases? (DONE)

Parametric mixed logit models exogenously assume the shape of the preference distribution, which is defined to be normal or log-normal in most cases. Thus, the analysis of preference heterogeneity is limited to that pre-defined shape. Nonparametric mixed logit models are unbiased of such assumptions and derive the (discrete) preference distribution in the base population endogenously from the data. This facilitates the analysis of more complex (discrete) distributions and, based on that, the segregation of preference groups with diverging characteristics. I added an explanation of the advantages and disadvantages in the paper.md.

Can you use English names instead of German names for the parameters in example_estimation.py? (DONE)

Yes, I changed the parameter names to English.

Can you either add the data for the Japanese market or drop the plots? They are not reproducible. (DONE)

I dropped the plots.

Can you remove dist folder from the repo? It makes the package unnecessarily large. (DONE)

I removed the dist folder.

Can you integrate automated runs of your tests scripts, e.g., to Github workflow? (DONE)

I integrated automated tests via GitHub actions. The respective GitHub action is working and executes the tests defined in the test-folder.

Could you modify the core module (or extract) such that the input data and model parameters can be directly provided either as numpy arrays or pandas dataframes? In its current application it cumbersome to integrate the package in an workflow since that data cannot be directly be given to the model.

I changed the functionality in the core module. Input data can now be provided via the keyword-argument "data_in" as pandas dataframes, which are loaded from .csv-files (or any other data format). The example_estimation.py script and the readme.txt have been adapted accordingly.

Can you explain the .picke files in ModelParam? Are they necessary for the flow? If so, could you use a human-readable datatype such as .json or .csv? In their current form it is not transparent how they integrate to the model.

The files in the folder ModelParam contain pre-estimated model parameters, which are required for the functionality of the simulation-routines (inherently based on pre-estimated logit models) and the estimation-routines. However, I substituted all .pickle-files by .csv files and adapted the code accordingly.

Could you add contribution guidelines to the repo? (OPEN)

This point is still open, I will work on it towards the end of the week.

Thanks again for your comments.

julianreul commented 1 year ago

Could you add contribution guidelines to the repo? (DONE)

I added a contributing.md file to the repo, according to my ideas of how to contribute.

l-kotzur commented 1 year ago

@sbenthall Unfortunately, I need to report a conflict of interest (COI) and withdraw from my review.

I have worked until last year in the same institute as @julianreul and co-authored articles with some of the contributors in this article.

I thought that my new role would not be seen as COI but the rules are quite clear on this situation "In addition, your recent (past year) association with the same organization of a submitter is a COI, for example, being employed at the same institution."

@julianreul I am sorry for this. Still, I hope that my feedback helped to improve the article.

julianreul commented 1 year ago

@l-kotzur Thanks for your feedback, it definitely helped to improve the submission!

I thought that my new role would not be seen as COI but the rules are quite clear on this situation "In addition, your recent (past year) association with the same organization of a submitter is a COI, for example, being employed at the same institution."

I should have checked these regulations more thoroughly before.

I hope we can find another reviewer soon.

sbenthall commented 1 year ago

Thanks for letting me know @l-kotzur .

@julianreul Do you have any other ideas for appropriate reviewers?

julianreul commented 1 year ago

Dear @sbenthall, I contacted a few potential reviewers, but received negative feedback.

Another potential reviewer would be Joe Molloy (Github-name: joemolloy), who developed the mixl-package at the ETH Zürich. The mixl-package is a framework for estimating discrete choice models, similar to MO|DE.behave, but written in R. Thus, it would be a good fit regarding the theoretical background.

sbenthall commented 1 year ago

Hello @joemolloy,

Would you be willing to review this submission to the Journal of Open Source Software?

Its author says you have written a similar library in R to this submission.

We are looking for a second reviewer after one of the reviewers backed out because of a conflict of interest.

Thanks, @sbenthall

julianreul commented 1 year ago

Hello @sbenthall,

today I received feedback from another researcher at the DLR (German Aerospace Center), who is willing to review our submission. Her name is Fabia Miorelli (GitHub-username: fabmio).

She is a PhD student at the DLR, working intensively with Python in the field of energy systems analysis and co-published another open-source model in Python, called "vencopy" (Gitlab: https://gitlab.com/dlr-ve/esy/vencopy/vencopy).

I would appreciate, if you nominate her as a reviewer to progress with the review-status.

Thanks, @julianreul

sbenthall commented 1 year ago

@julianreul Excellent. Thank you so much.

@fabmio would you be willing to review this submission to JOSS?

Submissions are reviewed according to the following criteria: https://joss.readthedocs.io/en/latest/review_criteria.html

If you accept, we will provide a checklist on this issue to support your review.

fabmio commented 1 year ago

Hi @sbenthall,

yes, I would be happy to review the submission.

julianreul commented 1 year ago

Dear @sbenthall,

if you agree to fabmio being a reviewer, I would be happy if you could nominate her as such, because she indicated to me, that she might start with her review during this week. Thank you.

Also, I wanted to ask @abhiramm7 on the progress of this review. I can understand, if other obligations are prioritized, but knowing would give the editor and the authors the chance to search for alternative reviewers?

Best Julian

sbenthall commented 1 year ago

@editorialbot remove @l-kotzur from reviewers

editorialbot commented 1 year ago

@l-kotzur removed from the reviewers list!

sbenthall commented 1 year ago

@editorialbot add @fabmio as reviewer

editorialbot commented 1 year ago

@fabmio added to the reviewers list!

sbenthall commented 1 year ago

Thank you @fabmio and @julianreul. @fabmio, please use @editorialbot generate my checklist to create a new reviewer checklist.

@abhiramm7 any update on this? Thank you.

fabmio commented 1 year ago

Review checklist for @fabmio

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

sbenthall commented 1 year ago

Hello @abhiramm7, we are still waiting on your review please.

@fabmio have you been able to review the documentation and functionality of the submission?

fabmio commented 1 year ago

@sbenthall I will be reviewing both missing parts in the upcoming week.

abhiramm7 commented 1 year ago

Review checklist for @abhiramm7

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

julianreul commented 1 year ago

@fabmio and @abhiramm7 thanks for starting your reviews! I am very much looking forward to your comments and the next steps in this review process.

abhiramm7 commented 1 year ago

Hi @julianreul , I've created two issues https://github.com/julianreul/mode_behave/issues/6 and https://github.com/julianreul/mode_behave/issues/5 in the project repository. Overall the package looks amazing. I just had a couple of minor comments, I hope you find them useful. Thanks! :D.

julianreul commented 1 year ago

Hi @abhiramm7, thanks for your encouraging feedback and your valuable comments! I am responding to your comments in the feed of the two issues https://github.com/julianreul/mode_behave/issues/5 and https://github.com/julianreul/mode_behave/issues/6

fabmio commented 1 year ago

Hi @julianreul, I have completed the first iteration of my review, below you can find some comments and ideas:

General checks

Functionality

Documentation

Software paper

I hope the suggestions are helpful to improve the package! :)

julianreul commented 1 year ago

Hi @fabmio,

thanks for your detailed review and great remarks! I have adressed all of your comments in recent commits. Please find a summary of the changes (bold) in response to your remarks (italic) below:

General checks

Functionality

Documentation

Software paper

Thanks again for your remarks and your willingness to review this piece of software. I hope to have addressed your comments sufficiently.

Best Julian

sbenthall commented 1 year ago

@fabmio and @abhiramm7 can you please comment on whether the author has adequately addressed your concerns? If you can complete the checklist or verbally sign off, that will let us proceed to the next stage of editing. If you have unaddressed concerns, please let @julianreul know.

abhiramm7 commented 1 year ago

@fabmio and @abhiramm7 can you please comment on whether the author has adequately addressed your concerns? If you can complete the checklist or verbally sign off, that will let us proceed to the next stage of editing. If you have unaddressed concerns, please let @julianreul know.

My comments have been addressed! Thanks for making the changes @julianreul

fabmio commented 1 year ago

@fabmio and @abhiramm7 can you please comment on whether the author has adequately addressed your concerns? If you can complete the checklist or verbally sign off, that will let us proceed to the next stage of editing. If you have unaddressed concerns, please let @julianreul know.

@sbenthall: All my comments have been addressed as well. I disagree with the arguments to the second point related to the code functionality. Nevertheless I deem it fine to continue with the publication process as the mentioned iterators are used in a limited scope and are not detrimental towards readability and maintainability of the code.

@julianreul: thank you for addressing all the comments!

sbenthall commented 1 year ago

Great!

@julianreul We can proceed to the next step of the process.

These are:

julianreul commented 1 year ago

Thank you @abhiramm7 and @fabmio for your reviews and @sbenthall for handling this submission up to this point.

I conducted a final proof read, tagged a release and created an archive on Zenodo:

sbenthall commented 1 year ago

@editorialbot set 10.5281/zenodo.8223829 as archive

editorialbot commented 1 year ago

Done! archive is now 10.5281/zenodo.8223829

sbenthall commented 1 year ago

@editorialbot set v1.0.12 as version

editorialbot commented 1 year ago

Done! version is now v1.0.12

sbenthall commented 1 year ago

Thanks for this submission @julianreul , and to the reviewers @fabmio and @abhiramm7

I recommend this submission for acceptance!

sbenthall commented 1 year ago

@editorialbot recommend-accept

editorialbot commented 1 year ago
Attempting dry run of processing paper acceptance...