search

openjournals / joss-reviews

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

[REVIEW]: mimosa: A Modern Graphical User Interface for 2-level Mixed Models #2116

Closed whedon closed 4 years ago

whedon commented 4 years ago

Submitting author: @johannes-titz (Johannes Titz) Repository: https://github.com/johannes-titz/mimosa Version: v.0.3.0 Editor: @cMadan Reviewer: @strengejacke, @aj2duncan Archive: 10.5281/zenodo.3775143

Status

status

Status badge code:

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

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

@strengejacke & @aj2duncan, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

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

✨ Please try and complete your review in the next two weeks ✨

Review checklist for @strengejacke

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @aj2duncan

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @strengejacke, @aj2duncan it looks like you're currently assigned to review this paper :tada:.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

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

@whedon commands

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

@whedon generate pdf
whedon commented 4 years ago 
Reference check summary:

OK DOIs

- 10.18637/jss.v067.i01 is OK
- 10.1080/0305498930190401 is OK
- 10.5281/zenodo.1308157 is OK
- 10.1016/0883-0355(89)90027-X is OK
- 10.1198/tas.2011.11077 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

cMadan commented 4 years ago

@strengejacke @aj2duncan, do let me know if you have any questions as you review the submission :)

aj2duncan commented 4 years ago

Hi @johannes-titz,

I've got a query for you if that's ok. In terms of contributions you list a contributer on mimosa.org and a second user has contributed 71 commits to the repo. I'm not sure if this is the same person or different people, but perhaps you need to consider the authorship of the submission?

Thanks,

Andrew

johannes-titz commented 4 years ago

@aj2duncan Thank you for the query! The contributor mentioned on mimosa.icu is Maria Reichert. If you run gitinspector -f R, you will see that she made one commit (the initial one), which makes about 6% of all changes. We planned to work on mimosa together and she wrote a first scaffold, but was too busy to continuously work on the project. She knows that I submitted the software to JOSS and is fine with not appearing as a main author. But I will add her as a contributor to the README.md and acknowledge her contribution--I just really forgot to do that before submitting.

The other user with 71 commits is actually just another account of myself (gitfortat). To my knowledge I cannot merge both accounts in such a way that these commits would belong to my new account. But the output of gitinspector at least shows my first name. The associated e-mail address for gitfortat is johannes.titz@gmail.com, to give an example here is one commit: https://github.com/johannes-titz/mimosa/commit/a716de17eb4c0a371cc5fd809036dac13fc0a0a6.patch

If you remove the ".patch" in the url you will see that this is a commit by gitfortat.

Sorry for the confusion!

strengejacke commented 4 years ago

Thanks @johannes-titz, the paper is well written and the website and readme are well documented. Beside @aj2duncan comment that you should think of adding other contributors either as "contributor" to the description-file, or even as co-author if this is justified, I have no further remarks regarding this review. However, I have a methodological remark. When including level-2 predictors as fixed effects, you might offer "de-meaning" variables, to avoid that the error terms correlate with covariates (which violates the iid-assumption of mixed models). You find some details here: https://easystats.github.io/parameters/reference/demean.html and in the references literature. I would not make this a requirement for this review, but I encourage you to think about considering "group-meaning" your level-2 predictors in your package.

strengejacke commented 4 years ago

Just read your comment, which makes my first part of my answer redundant.

aj2duncan commented 4 years ago

Hi @johannes-titz,

Thanks for the quick reply. That all seems fine and I can understand the dual accounts - sorry I should have dug a little deeper to see the email addresses!

I'll try to complete the review this evening.

Andrew

johannes-titz commented 4 years ago

@aj2duncan No problem. This is my mistake, sorry for not stating it earlier! I will add the information on the contributing author asap.

@strengejacke Thank you very much! I am actually not a big fan of centering since the data I usually analyze has a meaningful interpretation of the zero point. So far I thought centering is rather optional and more of a theoretical problem (what should zero represent?) than a statistical one. I was not familiar with the heterogeneity bias, so I will definitely read the suggested literature and consider adding a "demeaning" option.

I assume this is not the right place to discuss it, but I am curious: Will demeaning only affect p-values and confidence intervals (e.g. R² remains the same)? Could the problem alternatively be handled by bootstrapping (if it only affects the error terms)? What happens with variables that are on a ratio scale? From a theoretical point of view, I would not suggest transforming them (except for a multiplication by a constant).

Since you do not insist on adding a demeaning option in this review, I would like to consider it for a later release. But thank you very much for pointing me to this problem! I am looking forward to read about it in more detail.

aj2duncan commented 4 years ago

Hi @johannes-titz,

As @strengejacke says, the paper is well written and everything is well documented. The app is intuitive and it is very nice that it is hosted online for users. The only minor comments I would make are related to Shiny and I have no further comments about the paper.

In terms of the app I can see two small things to consider for improvements. First is to consider adding a progress bar to the model calculation, even in the example you can get the app to hang slightly if you click the wrong variable. Adding a progress bar would let the user know it is just working away in the background. Second is a recommendation to consider whether you want the app to recalculate the model anytime a user changes an input. You may wish to isolate some (or all) inputs and let the user only rerun the model when they've finished. You will know the intended audience far better than I and can make an informed decision whether either of these suggestions merit the time needed to implement them.

strengejacke commented 4 years ago

That's a good recommendation, indeed. I didn't run into performance problems, so I didn't realize that issue - but having larger data sets, it might make more sense to select your input variables first, then click on an "update-button" or similar, which might be useful in particular for very large data sets.

johannes-titz commented 4 years ago

@aj2duncan, @strengejacke Thank you for very much for the feedback! I really like the idea of having a progress bar. Unfortunately this is not a trivial think to do (e.g. see discussion here https://github.com/lme4/lme4/issues/482). It would be easier to show the iteration step and the optimization criterion, which is sort of a progress indicator (e.g. one can anticipate when it converges). But even for this I first need to check how I can implement this. Isolating the inputs is also a good idea, although in most cases I prefer the interactivity. Maybe I can add an option to isolate/not isolate the inputs. I will try it out and give you an update.

arfon commented 4 years ago

Dear authors and reviewers

We wanted to notify you that in light of the current COVID-19 pandemic, JOSS has decided to suspend submission of new manuscripts and to handle existing manuscripts (such as this one) on a "best efforts basis". We understand that you may need to attend to more pressing issues than completing a review or updating a repository in response to a review. If this is the case, a quick note indicating that you need to put a "pause" on your involvement with a review would be appreciated but is not required.

Thanks in advance for your understanding.

Arfon Smith, Editor in Chief, on behalf of the JOSS editorial team.

johannes-titz commented 4 years ago

@aj2duncan, @strengejacke I had some time to check out how to implement the suggested features and would like to give you an update:

First of all, a progress bar is quite hard to implement for two reasons: (1) It is not easy to predict how many iteration steps are necessary for convergence and (2) At the moment I do not know how to access the iteration steps without “hacking” lme4 (I asked a question on SO, so maybe I will find away in the future). I thought of just displaying the convergence criterion, but because of point (2) (and shiny limitations), this is also not possible at the moment. Unfortunately I cannot invest much more time into this as I already spend several days to research and try out some options. Instead, I simply added a “spinner“ and a message that the model is estimated. I believe this is sufficient since all of the plausible models I fitted finished in a few seconds.

The only problem arose when I used “student” as an independent variable for the example data set. The variable has 650 levels and setting up the model takes pretty much forever, while estimating goes quickly. It does not make sense to fit a model with a factor that has so many levels. Thus, I decided to exclude factor variables with more than 10 levels. I think this is reasonable as it avoids fitting meaningless models that take very long. Furthermore, if one still wants to do this, one can create dummy variables manually before uploading the data to mimosa.

Regarding the non-reactive (isolated) inputs, I simply included a checkbox that switches between isolated and non-isolated inputs. If the inputs are isolated, a button to estimate the model is displayed. Depending on user feedback, I might change the default to isolated inputs in the future. For now I would like to keep it reactive when mimosa starts.

The updated version is currently available under the branch “joss_review”; to install in R: devtools::install_github("johannes-titz/mimosa@joss_review")

The updated app is also available under www.mimosa.icu/joss_review and for the example data set: https://www.mimosa.icu/joss_review_example. If you are happy with the changes, I will merge them into master. I hope everything works as expected.

By the way, I also updated the acknowledgements and added the contributing author to the DESCRIPTION file (this is also already on master).

@cMadan If it is ok I would like to continue the review and not pause it.

aj2duncan commented 4 years ago

@johannes-titz I really like those changes that you've made. The spinner that you've added is just exactly the thing that I was thinking about, rather than something more accurate using the convergence of lme4. I also like the reactive vs non-reactive mode. Very simple to use. Changing between the two settings did also show/hide the contributions, which you may want to tweak.

Otherwise, really like the changes. @cMadan from my point of view - very happy for this review to be concluded.

johannes-titz commented 4 years ago

@aj2duncan Thank you very much! Regarding the problem with showing/hiding the contributions, I am not able to reproduce it. When exactly does it happen? Which browser are you using?

strengejacke commented 4 years ago

@cMadan @johannes-titz From my perspective, all issues have been addressed, as well.

aj2duncan commented 4 years ago

@johannes-titz I was using your example app at the joss-review link above on Chrome but now I can't reproduce it either - so nevermind not sure what was happening.

cMadan commented 4 years ago

@strengejacke @aj2duncan, thank you both for your thorough reviews of this submission!

@johannes-titz, next I will do some final checks.

johannes-titz commented 4 years ago

@aj2duncan Ok. I guess it can happen if there are many variables and the footer is moved down because of this, but I am not really sure.

@cMadan Thank you! If I can help somehow, let me know. The master is now up to date. If you think copy-editing for the paper is necessary (as you can see I am not a native speaker), I will look for someone.

cMadan commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

johannes-titz commented 4 years ago

@cMadan Just to keep you up to date: I sent the paper to a copy-editor and she found a couple of small grammar mistakes and made some minor improvements in word choice. These changes are already included in the master (see: https://github.com/johannes-titz/mimosa/commit/a80736f5442ff506cb489d937b542b1fb91f3ad0?short_path=fb6416b). I believe this is now good to go.

@whedon generate pdf

danielskatz commented 4 years ago

@whedon generate pdf

⬆️ commands to whedon need to be the start of a comment ⬆️

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

arfon commented 4 years ago

:wave: @cMadan - looks like this is very close to being ready to accept?

cMadan commented 4 years ago

@whedon check references

whedon commented 4 years ago 
Reference check summary:

OK DOIs

- 10.18637/jss.v067.i01 is OK
- 10.1080/0305498930190401 is OK
- 10.5281/zenodo.1308157 is OK
- 10.1016/0883-0355(89)90027-X is OK
- 10.1198/tas.2011.11077 is OK

MISSING DOIs

- None

INVALID DOIs

- None
cMadan commented 4 years ago

@johannes-titz, everything looks good to me!

To move forward with accepting your submission, there are a few last things to take care of:

You may find this helpful: https://guides.github.com/activities/citable-code/

johannes-titz commented 4 years ago

@cMadan: Thank you! Here comes the new tagged version and the zenodo archive: v.0.3.0: https://github.com/johannes-titz/mimosa/tree/v0.3.0 Zenodo doi: 10.5281/zenodo.3775143 (changed title and author list, added ORCID)

cMadan commented 4 years ago

@whedon set 10.5281/zenodo.3775143 as archive

whedon commented 4 years ago

OK. 10.5281/zenodo.3775143 is the archive.

cMadan commented 4 years ago

@whedon set v.0.3.0 as version

whedon commented 4 years ago

OK. v.0.3.0 is the version.

cMadan commented 4 years ago

@whedon accept

whedon commented 4 years ago 
Attempting dry run of processing paper acceptance...
whedon commented 4 years ago 
Reference check summary:

OK DOIs

- 10.18637/jss.v067.i01 is OK
- 10.1080/0305498930190401 is OK
- 10.5281/zenodo.1308157 is OK
- 10.1016/0883-0355(89)90027-X is OK
- 10.1198/tas.2011.11077 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 4 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/1436

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/1436, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true
arfon commented 4 years ago

I'll take this from here. Thanks @cMadan.

arfon commented 4 years ago

@whedon accept deposit=true

whedon commented 4 years ago 
Doing it live! Attempting automated processing of paper acceptance...
whedon commented 4 years ago

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

whedon commented 4 years ago

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/1437
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.02116
  3. If everything looks good, then close this review issue.

  4. Party like you just published a paper! 🎉🌈🦄💃👻🤘

    Any issues? notify your editorial technical team...

arfon commented 4 years ago

@strengejacke, @aj2duncan - many thanks for your reviews here and to @cMadan for editing this submission ✨

@johannes-titz - your paper is now accepted into JOSS :zap::rocket::boom:

whedon commented 4 years ago

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://joss.theoj.org/papers/10.21105/joss.02116/status.svg)](https://doi.org/10.21105/joss.02116)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.02116">
  <img src="https://joss.theoj.org/papers/10.21105/joss.02116/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.02116/status.svg
   :target: https://doi.org/10.21105/joss.02116

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

johannes-titz commented 4 years ago

@strengejacke @aj2duncan Thank you very much for the reviews! @cMadan Thanks a lot for the editing work. @arfon Many thanks for the last steps.