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
[REVIEW]: Madym: A C++ toolkit for quantitative DCE-MRI analysis #3523
Comments
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @agahkarakuzu, @matteomancini it looks like you're currently assigned to review this paper 🎉. Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post. ⭐ Important ⭐ 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:
For a list of things I can do to help you, just type:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
Wordcount for |
|
|
👋 @agahkarakuzu, please update us on how your review is going (this is an automated reminder). |
👋 @matteomancini, please update us on how your review is going (this is an automated reminder). |
I have started working on my revision and I should be done by the 27th of August. |
Thank you, @matteomancini ! If you have any difficulties in completing your review this week, please just let me know. Otherwise, as you're working through the checklist above, please add a pointer to this issue on any issues you may open on the software repo so they are cross-linked. |
👋 Hi @agahkarakuzu -- have you had a chance to take a look at the paper and start working through the review checklist ? If you're facing any issues here, please let me know. |
Hi @emdupre I'll finish my review this week, sorry for the delay. Thank you for the reminder! |
Thank you so much @michaelberks for the amazing codebase and your patience as it took me a while to give this review a stab. My review will follow the principles as they are outlined in the paper. Ease-of-useI was able to find everything a user may need quite easily, from this video to a guideline on how to run example scripts. GUI availability makes this tool really attractive for clinical scientists! SuggestionI think having an easier to find layman download instructions would make it even more attractive. For example instead of Installing from pre-built binaries, a separate DOWNLOAD MADYM (listing different OS) section appearing earlier would help non-programming users. Minor issueI was able to successfully build target doxygen API docs, I guess it is a matter of website deployment (or access on my end) issue.
I was able to find them without requesting, rewording would be helpful. Reproducible researchTransparency and the organization of fitting configs are laudable! I am yet to test the user experience for dealing with these configurations across interfaces, but this is a really critical issue, nicely addressed by this software. Some other technical aspects of a reproducible codebase are nicely backed up by a through CI/CD pipeline and containers (thank you so much for sharing Dockerfile(s)). I am happy to see MR-Hub credited in the paper and the software is listed there, solving the second half of the reproducibility problem (social) by reaching out to the target crowd. SuggestionNothing to add. I will add the remaining of my review (Extensibility and Performance) after testing the software both using the pre-compiled binaries and the binaries I compiled. I was not able to pass Not sure if testing all the wrappers is within the scope of this review, but I'll give it a try anyway. |
@agahkarakuzu thank you very much for the kind words, and very helpful review. I will try to respond fully to some other of the other points in the next couple of weeks (I'm pretty tied up for the next few days, then on leave for a week from Friday), however I've fixed the first minor issue... The doxygen link in the wiki was pointing to an old link on my university pages, before I worked out how to use GitLab's awesome pages feature to automatically generate doxy docs during CI/CD. The doxygen documentation is now here, and I have corrected the link in the library wiki here Re the wrappers. I need to update the paper as I have changed how madym interacts with the python wrapper. The python wrapper is now included directly with the C++ source code, with instructions on the wiki how to use. The link in the paper points to an obsolete repository, the contents of which have been moved to a new project QbiPy here. However the user doesn't need to access QbiPy directly because it is available on PyPi and will be automatically installed as a dependency of the included python wrapper. The Matlab wrapper is as described in its own repo. I totally understand if trying them is beyond the scope of this review, but if you did try them I'd really appreciate the feedback - they've been used internally in the lab, but I don't think anyone outside the lab has tried them yet so it'd be great to know how easy you find them to set up. Python in particular I've tried to make the installation seamless, but things always seem to go wrong with conda environments, pip install etc! If you need any further info/help re the Qt/ Thanks again for your time in reviewing the code. |
Thank you @michaelberks for working on this very interesting project. This software is an interesting contribution to the landscape of quantitative MRI tools, and for sure the first tool I would consider trying out if I was given DCE-MRI data. I will follow the JOSS checklist for this review. All the general checks are in place. In terms of functionality, I've compiled Madym on macOS ( In terms of functionality, I managed to immediately run the example analysis on the provided data using the related script for the command-line case. For the GUI, the instructions provided in the wiki are also quite detailed. The testing process is automated and immediate to run as well. In terms of the software paper, it is well written and to the point. The summary should be enough to explain it to a non-specialistic audience, although a general reference (if the reference list length allows it) about DCE-MRI would help an inexperienced reader. The statement of need makes clearly the point of the purpose of this project, but what is currently missing is the current state of the field: from the paper it is not currently clear if Madym is the first of its kind or if there are other open-source alternatives to perform similar analyses. If the length of the paper is already at the limit, it should be possible to shorten the statement of need to save enough words to fill the current state gap. Once again, I think this is great work and a useful addition to qMRI tools, so once these minor issues are fixed I looked forward to see this features in JOSS! |
I was able to test the software, so I am moving on with my review following the same structure above: PerformanceBelow are the logs from Madym I built from sources (command line):
Following the processing, I obtained the following
SuggestionsNone. Extensibility & InteroperabilityThe organization of Python interfaceAs this has become a project of its own, I won't incorporate a review on QbiPy here. On a quick note, this link provided in Matlab interfaceI started with General commentLooks like the main project is split between several repos and documentation pages (and other fies). I suggest centralizing the documentation and removing docx etc. documentation resources from individual repositories to make it easier for users to follow. Comments specific to the Matlab wrapperIf I understood correctly, part of madym_matlab are actually interfacing with the Other than that, I was able to use the matlab wrappers to run Regarding configuration/log/protocol files and map units
I agree with @matteomancini, most of the open-source qMRI software has a lot in common with some overlapping functionality, but each differ in application focus, language choice, performance etc. You can see PyQMRI's JOSS article as an example for a statement on how Madym differs from other existing open-source qMRI software. It is fascinating that the JOSS is accommodating more and more qMRI software articles (QUIT, qMRLab, PyQMRI) and I am looking forward to seeing Madym added to that list following this revision. Thank you for the great work @michaelberks! |
@matteomancini thank-you very much for adding your review and the helpful suggestions, and to @agahkarakuzu for the detailed update. As previously mentioned, I'm away on leave from tomorrow for a week, but on my return I will:
Thanks again for taking the time to make such helpful feedback, it really is appreciated. |
👋 Hi @michaelberks, I just wanted to check-in on this and make sure you weren't facing any logistical blocks. If there's something I can provide to help in the review process, please let me know. |
Hi @emdupre apologies for delay, I was off work for a few days when I got back from leave and then had some conference deadlines to meet. Summarising the main points raised, I will be fixing the following: Minor fixes
Changes requiring more major work
I should be able to complete the minor fixes today and will update when done. Please let me know if you think there is anything I have missed. |
I have now completed the above tasks. Some more details below Program logsFor detailing how acquisition parameters are set from XTR files, the program logs now look something like this: T1 mapping
TK model fitting
Related workI have added a new Related Work section in the paper.md describing other open-source DCE-MRI packages and Madym's relationship to them. BIDSRegarding incorporating BIDS into Madym, I have added the following issue on the main project issue board. SummaryI have made the above changes on a new branch 61-joss-review-fixes, but have not yet merged this branch with main, and so have not yet triggered new binary builds. If ok, I will delay the merge until the review is complete, so we don't have unnecessary version increments. Please let me know if there are any other changes you would like to see me make. |
@michaelberks thank you for addressing the issues I raised, I have checked all the boxes following your update. I did not know about some of the tools you mentioned in the related work section, discovering that there are more open-source qMRI projects than I knew always cheers me up! I am glad that BIDS compatibility is now on the to-do list of your project, I hope that it will foster interoperability of publicly available (q)MRI software in the years to come. 🟢 Upon the latest changes @michaelberks made, my review is completed. I give my green light to this article for publication in JOSS. Once again, thank you for the amazing job! |
Thank-you again for your very helpful review comments @agahkarakuzu |
@michaelberks thanks for addressing most comments. The only things currently missing are the following:
|
Thank you again, @agahkarakuzu and @matteomancini for your thoughtful reviews and @michaelberks for your impressive work on At this point could you please:
I can then move forward with accepting the submission 🚀 |
@whedon generate pdf from branch 61-joss-review-fixes |
|
@whedon check references from branch 61-joss-review-fixes |
|
|
Thanks @emdupre. I have merged the JOSS fixes branch back with master, and then made a new release v4.15.2. This has been archived at zenodo here, with DOI 10.5281/zenodo.5554771. |
@whedon set v4.15.2 as version |
OK. v4.15.2 is the version. |
Thank you, @michaelberks ! One confirmation on the Zenodo metadata : The archive (Madym: quantitative analysis software for perfusion-MRI) and the JOSS software paper (Madym: A C++ toolkit for quantitative DCE-MRI analysis) have two slightly different titles. Would it be possible to harmonize these ? |
Done. I've updated the archive title to "Madym: A C++ toolkit for quantitative DCE-MRI analysis" to match JOSS. |
@whedon set 10.5281/zenodo.5554771 as archive |
OK. 10.5281/zenodo.5554771 is the archive. |
@whedon recommend-accept |
|
|
👋 @openjournals/joss-eics, this paper is ready to be accepted and published. Check final proof 👉 openjournals/joss-papers#2652 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2652, then you can now move forward with accepting the submission by compiling again with the flag
|
@whedon accept deposit=true |
|
🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦 |
🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨 Here's what you must now do:
Any issues? Notify your editorial technical team... |
Congratulations @michaelberks on your article's publication in JOSS! Many thanks to @agahkarakuzu and @matteomancini for reviewing this, and @emdupre for editing. |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippets:
This is how it will look in your documentation: 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:
|
Submitting author: @michaelberks (Michael Berks)
Repository: https://gitlab.com/manchester_qbi/manchester_qbi_public/madym_cxx/
Version: v4.15.2
Editor: @emdupre
Reviewer: @agahkarakuzu, @matteomancini
Archive: 10.5281/zenodo.5554771
Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.
Status
Status badge code:
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
@agahkarakuzu & @matteomancini, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @emdupre 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 ✨
Review checklist for @agahkarakuzu
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
Review checklist for @matteomancini
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
The text was updated successfully, but these errors were encountered: