Skip to content
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]: MDL Suite: A language, generator and compiler for describing mazes #1815

Open
whedon opened this issue Oct 16, 2019 · 58 comments
Open

[REVIEW]: MDL Suite: A language, generator and compiler for describing mazes #1815

whedon opened this issue Oct 16, 2019 · 58 comments
Assignees
Labels

Comments

@whedon
Copy link
Collaborator

@whedon whedon commented Oct 16, 2019

Submitting author: @akashnag (Akash Nag)
Repository: https://github.com/akashnag/MDL-Suite
Version: 1.0
Editor: @drvinceknight
Reviewer: @rreinecke, @gradvohl
Archive: Pending

Status

status

Status badge code:

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

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

@rreinecke & @gradvohl, 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 @drvinceknight know.

Please try and complete your review in the next two weeks

Review checklist for @rreinecke

Conflict of interest

  • I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@Akash-Nag) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

Review checklist for @gradvohl

Conflict of interest

  • I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@Akash-Nag) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Oct 16, 2019

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

⭐️ 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:

  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

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Oct 16, 2019

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Oct 16, 2019

@rreinecke

This comment has been minimized.

Copy link
Collaborator

@rreinecke rreinecke commented Oct 16, 2019

@Akash-Nag Please fix akashnag/MDL-Suite#3 (and akashnag/MDL-Suite#2 ) to enable me to further review your submission. Thanks!

@rreinecke

This comment has been minimized.

Copy link
Collaborator

@rreinecke rreinecke commented Oct 17, 2019

@Akash-Nag please address the following issues akashnag/MDL-Suite#5 akashnag/MDL-Suite#6 . Especially the automated tests will help to understand your code better. Without them I'm currently not able to proceed with the review. Please also consider improving your code documentation further.
If you have questions please feel free to ask in the according issues.

@danielskatz

This comment has been minimized.

Copy link

@danielskatz danielskatz commented Nov 2, 2019

👋 @Akash-Nag - can you update us on this review and your progress in addressing the issues discussed above?

@akashnag

This comment has been minimized.

Copy link

@akashnag akashnag commented Nov 3, 2019

@gradvohl

This comment has been minimized.

Copy link
Collaborator

@gradvohl gradvohl commented Nov 3, 2019

Regarding the docs, I think the author solved this issue. However, when I tried to run, I still get the following error:

Exception in thread "main" java.lang.UnsupportedClassVersionError: mdlg/MDLG has been compiled by a more recent version of the Java Runtime (class file version 54.0), this version of the Java Runtime only recognizes class file versions up to 52.0
        at java.lang.ClassLoader.defineClass1(Native Method)
        at java.lang.ClassLoader.defineClass(ClassLoader.java:756)
        at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
        at java.net.URLClassLoader.defineClass(URLClassLoader.java:468)
        at java.net.URLClassLoader.access$100(URLClassLoader.java:74)
        at java.net.URLClassLoader$1.run(URLClassLoader.java:369)
        at java.net.URLClassLoader$1.run(URLClassLoader.java:363)
        at java.security.AccessController.doPrivileged(Native Method)
        at java.net.URLClassLoader.findClass(URLClassLoader.java:362)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:418)
        at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:352)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:351)
        at sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:495)

Should I run it with a specific java version? If so, this should be highlighted in the README.md. as well as in the docs.

@akashnag

This comment has been minimized.

Copy link

@akashnag akashnag commented Nov 4, 2019

@rreinecke

This comment has been minimized.

Copy link
Collaborator

@rreinecke rreinecke commented Nov 4, 2019

All the issues raised by the reviewers have been resolved, except for the Tests. Incorporating JUnit tests is difficult for a project of this type since this is not a library that can be invoked. Instead, it is a tool whose output is an image file. Obviously, checking whether the generated maze-image conforms to the input specifications requires visual inspection or outlandish AI-driven image recognition algorithms beyond the scope of this project. For internal functions which do not generate the image directly, it may be possible to write tests but the difficulty there lies with the fact that most of these functions work with matrices, and again, checking whether these maze matrices conform to the specifications will require their own algorithms that are unknown to me. The code for the tests will, in all likelihood, be larger and more complicated than the source code itself. Therefore, I would request you to kindly proceed with the review for this project as it stands now. For your reference, I have made the following changes: 1. The readme now contains detailed installation and build instructions, names of contributors, etc. 2. The JavaDocs has been made available 3. A shell script has been provided for automated generation of random mazes, for testing purposes

On Sat, 2 Nov 2019, 19:25 Daniel S. Katz, @.***> wrote: wave @Akash-Nag https://github.com/Akash-Nag - can you update us on this review and your progress in addressing the issues discussed above? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1815?email_source=notifications&email_token=ALUWTRYXNIHROLNTRD524ILQRWBGRA5CNFSM4JBKNNM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC44OOQ#issuecomment-549046074>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALUWTR5C3PRR57OB2Z23IZDQRWBGRANCNFSM4JBKNNMQ .

@drvinceknight @danielskatz I cannot confirm the statement given by @Akash-Nag . Yes all other concerns except the community guidelines and tests have been addressed. However, I do not agree with the assessment that unit tests are impossible and I find it bewildering that our suggestions as reviewers are fully ignored. I understand that JOSS has a very constructive review process, thus I opened an issue more than two weeks ago suggesting unit tests and that I would be happy to provide additional inputs if necessary. This was completely ignored by @Akash-Nag till the statement above was given. As a computer scientist I cannot agree that test are not possible. Leaving the image generation aside, the software contains a maze generator that is able to produce random mazes. It is unclear to me if they are always solvable? If so, how do you prove that? Can they also be unsolvable? It seems you are using recursive-division to generate them. Then at least test the method that is implementing that algorithm. @Akash-Nag writes that the software is intended to be used for research. IF that is the case it needs to be clear how the software works. You can only ensure this by providing test cases that verify this behavior.

@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Nov 4, 2019

@Akash-Nag I fully agree with @rreinecke and do not feel that tests are impossible. Is this something you will be able to address?

@akashnag

This comment has been minimized.

Copy link

@akashnag akashnag commented Nov 5, 2019

@rreinecke

This comment has been minimized.

Copy link
Collaborator

@rreinecke rreinecke commented Nov 5, 2019

I apologize for any misunderstanding that may have occurred. I never intended to say that tests would be impossible, but only very difficult given the type of the project. Furthermore, I was not sure which aspects of the project could be actually tested as I was sure that neither the maze image generation nor the path markers could be feasibly tested to ensure they conform to the specifications. Due to this confusion I was not able to reply to the issue, but in restrospect I think I should have asked for more clarification, and I am very sorry for that. However, now that you mention about the proof of solvability of the generated mazes, I think there will be two ways of going about it. Firsly, there can be a formal proof of the correctness of the algorithm in the paper itself, but I think the format of JOSS papers will not be able to accomodate that. Secondly, I can include a function which, given a generated maze matrix, will print out any one path (as a list of coordinates) from the entrance to the exit, or alternately, a test which will return true/false depending on whether or not a path exists. Kindly state which do you want.

On Mon, 4 Nov 2019, 23:12 Vince Knight, @.***> wrote: @Akash-Nag https://github.com/Akash-Nag I fully agree with @rreinecke https://github.com/rreinecke and do not feel that tests are impossible. Is this something you will be able to address? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1815?email_source=notifications&email_token=ALUWTR4NUO5LSGGWS5SVVN3QSBNHJA5CNFSM4JBKNNM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDADEYI#issuecomment-549466721>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALUWTRZTHEIKXYDJ2ZPJURDQSBNHJANCNFSM4JBKNNMQ .

@Akash-Nag please see akashnag/MDL-Suite#5 for reply and further discussion.

@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Nov 28, 2019

@Akash-Nag any update on this?

@rreinecke

This comment has been minimized.

Copy link
Collaborator

@rreinecke rreinecke commented Nov 28, 2019

@Akash-Nag any update on this?

@drvinceknight I've asked him the same last week and it seems that he is very busy and currently not able to complete the suggested changes. I've suggested a re-submission to him but advised him to ask the handling editor (aka you) about how to handle this.

@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Nov 28, 2019

Thanks, I'll wait to hear from them to confirm before I close this.

@openjournals/jose-eics can I confirm the process for closing ("rejecting for now") a submission? Is it as simple as closing this issue or is there some @whedon magic I should use instead?

@labarba

This comment has been minimized.

Copy link
Member

@labarba labarba commented Nov 28, 2019

You can add the "paused" label

@labarba labarba added the paused label Nov 28, 2019
@rreinecke

This comment has been minimized.

Copy link
Collaborator

@rreinecke rreinecke commented Nov 28, 2019

@drvinceknight for tracking completed reviews via e.g. publons this labeling is not really satisfying. If this project is never revived this does not count as a review. I'm very happy to review for JOSS anytime but it would be nice if you guys would appreciate that work with a completed review mail that at least allows to track that effort.

@akashnag

This comment has been minimized.

Copy link

@akashnag akashnag commented Nov 29, 2019

@drvinceknight kindly advise regarding the process of resubmission. I'm sorry for not being able to make the necessary changes right now as I am very busy with too many unforeseen assignments. I apologize for the inconvenience caused. Please advise whether I should withdraw the paper (from JOSS website) and then resubmit later? Or, can/should this review be just paused, etc. in some way?

@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Nov 30, 2019

@rreinecke I understand completely. Thank you again for your time and work reviewing the paper. It's greatly appreciated.

@rreinecke and @Akash-Nag I am just clarifying the process myself via email, I will get back to you here as soon as I'm confident in the correct path forward.

@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Dec 2, 2019

Hi @rreinecke and @Akash-Nag.

Thank you for your patience.

I completely realise the effort and need for recognition of said effort of reviewers @rreinecke so once again; thank you and hopefully I can explain the options available so that we can choose the best one that importantly works for @Akash-Nag and the paper.

Option 1: The paper is withdrawn. This is the best option if @Akash-Nag is unlikely to come back to this in the next 3 months or so.

Option 2: We leave the paused option - this is best if @Akash-Nag is likely to come back to this in the next 3 months or so. @rreinecke I can set @whedon up to set a reminder so that we all do not forget, in which case in 3 months time we can revisit and if @Akash-Nag is unlikely to make progress we can go with option 1 at that stage.

Which way forward is preferable at this stage?

@akashnag

This comment has been minimized.

Copy link

@akashnag akashnag commented Dec 3, 2019

@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Dec 3, 2019

Assuming this is fine with you @rreinecke then the review is marked as pause. I will set a reminder for all of us of 3 months.

@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Dec 3, 2019

@whedon remind @Akash-Nag in three months

@akashnag

This comment has been minimized.

Copy link

@akashnag akashnag commented Feb 4, 2020

@drvinceknight drvinceknight removed the paused label Feb 5, 2020
@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Feb 5, 2020

I have removed the pause label.

@rreinecke

This comment has been minimized.

Copy link
Collaborator

@rreinecke rreinecke commented Feb 5, 2020

@Akash-Nag Well done. See akashnag/MDL-Suite#5 for my reply.

@rreinecke

This comment has been minimized.

Copy link
Collaborator

@rreinecke rreinecke commented Feb 5, 2020

@Akash-Nag looks like we are almost done from my side. See akashnag/MDL-Suite#7 and akashnag/MDL-Suite#8

@rreinecke

This comment has been minimized.

Copy link
Collaborator

@rreinecke rreinecke commented Feb 6, 2020

@whedon generate pdf

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Feb 6, 2020

@rreinecke

This comment has been minimized.

Copy link
Collaborator

@rreinecke rreinecke commented Feb 8, 2020

@drvinceknight @Akash-Nag all done and accepted from my side.

@gradvohl

This comment has been minimized.

Copy link
Collaborator

@gradvohl gradvohl commented Feb 9, 2020

From my side is also ok. Congrats.

@akashnag

This comment has been minimized.

Copy link

@akashnag akashnag commented Feb 9, 2020

@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Feb 10, 2020

@whedon generate proof

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Feb 10, 2020

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@whedon commands
@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Feb 10, 2020

@whedon check references

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Feb 10, 2020

Reference check summary:

OK DOIs

- None

MISSING DOIs

- https://doi.org/10.1109/confluence.2014.6949354 may be missing for title: Survey on techniques used in autonomous maze solving robot
- https://doi.org/10.3758/brm.40.1.353 may be missing for title: Maze Suite 1.0: A complete set of tools to prepare, present, and analyze navigational and spatial cognitive neuroscience experiments

INVALID DOIs

- None
@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Feb 10, 2020

@whedon generate pdf

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Feb 10, 2020

@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Feb 10, 2020

@Akash-Nag it looks like you are missing two DOIs:

@akashnag

This comment has been minimized.

Copy link

@akashnag akashnag commented Feb 11, 2020

@akashnag

This comment has been minimized.

Copy link

@akashnag akashnag commented Feb 11, 2020

This is to inform you that I have updated my username (reclaimed my older profile), and I had already informed @arfon that I was going to do so. For the time being, GitHub will automatically redirect my old repository for this review at: https://github.com/Akash-Nag/MDL-Suite to the new repository at: https://github.com/akashnag/MDL-Suite till any new user claims my now-defunct username and creates a new repository with the same name. I kindly request the journal admin to update my repository URL for this paper to: https://github.com/akashnag/MDL-Suite so that any inadvertent links to a different repository do not occur in future.

Thanks

@arfon

This comment has been minimized.

Copy link
Member

@arfon arfon commented Feb 11, 2020

@akashnag done.

@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Feb 11, 2020

@whedon check references

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Feb 11, 2020

Reference check summary:

OK DOIs

- None

MISSING DOIs

- None

INVALID DOIs

- https://doi.org/10.1109/CONFLUENCE.2014.6949354 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.3758/BRM.40.1.353 is INVALID because of 'https://doi.org/' prefix
@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Feb 11, 2020

Sorry @akashnag but you haven't added the DOIs correctly, the DOI for the first one is actually just 10.1109/confluence.2014.6949354 here's a full example of a bib file if that's helpful: https://github.com/drvinceknight/Nashpy/blob/master/paper.bib

@akashnag

This comment has been minimized.

Copy link

@akashnag akashnag commented Feb 12, 2020

@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Feb 12, 2020

@whedon check references

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Feb 12, 2020

Reference check summary:

OK DOIs

- 10.1109/CONFLUENCE.2014.6949354 is OK
- 10.3758/BRM.40.1.353 is OK

MISSING DOIs

- None

INVALID DOIs

- None
@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Feb 12, 2020

@whedon generate pdf

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Feb 12, 2020

@drvinceknight

This comment has been minimized.

Copy link
Collaborator

@drvinceknight drvinceknight commented Feb 12, 2020

Ok everything looks good to me.

@akashnag could you make a tagged release and archive. Once you've done that can you report the version number and archive DOI here.

Please make sure that the archive deposit has the correct metadata (title and author list).

@danielskatz

This comment has been minimized.

Copy link

@danielskatz danielskatz commented Feb 12, 2020

@drvinceknight - before you turn this over to the EiCs, please also proof-read the paper and references and make sure you are satisfied with them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.