-
Notifications
You must be signed in to change notification settings - Fork 7
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
[JOSS] Functionality documentation #129
Comments
@EmilyBourne Thanks for pointing this out. DisCoTec was indeed missing in the list of software. This does not solve the API documentation issue, but I added DisCoTec on sparsegrids.org. |
@obersteiner , @vancraar and me are currently putting the finishing touches on the new documentation at https://discotec.readthedocs.io/ ! At the same time, the main README is getting significantly shorter, see here: https://github.com/SGpp/DisCoTec/tree/feature_documentation . But it now also tells you "When to (not) use DisCoTec" :) @EmilyBourne @jakelangham we'll continue hacking on it for a few more days and let you know when we think we are "done" -- nonetheless, feel free to leave any intermediate feedback! |
@EmilyBourne @jakelangham I believe the documentation is fine for review! -> https://discotec.readthedocs.io/ with a few edits pending, which can be followed in #133 Please do let us know if things are unclear, your "outside" perspective can really help us make this a great documentation :) |
Further to @EmilyBourne's initial comment - there should be a link to the new docs somewhere in the README.md that's at least as prominent as the sparsegrids webpage link. |
Also to address this bullet point in the JOSS review: 'Do the authors clearly state what problems the software is designed to solve and who the target audience is?' I think you could be a little clearer, either in the docs or the README. It is up to you how you do this but one suggestion would be to add some more text to the 'What is Discotec?' section that explains/advertises the benefits of using it and perhaps suggests the sorts of applications it might be particular suited for. |
I am trying something -- can you spot it? |
@jakelangham I thought that the "When (not) to use DisCoTec" sections could fulfill this function, but if you didn't find it, it's obviously not prominent enough yet (or not what you were looking for?). Maybe it would be more logical to put above the installation section anyways... |
@freifrauvonbleifrei Ah, I had not fully appreciated yesterday that I should be reviewing the feature_documentation branch... What you've written looks ideal and just what I was looking for. It could go a bit higher up in the README but that's up to you. p.s. It might be a requirement for JOSS that these commits are merged into main before publication. |
@freifrauvonbleifrei I've finished reading through the documentation. The effort to produce this is greatly appreciated, but I think it could be improved in places, so here are my comments - a mixture of minor things and broader ideas that I think would help. Thanks.
|
I feel some inner resistance to actually introduce the solver here, because I think it would distract from the main point I am trying to make. What is your opinion on replacing "advection solver" with "memory-bandwidth-bound 6-D PDE solver" (or similar) in this section @jakelangham ? |
(addresses 6th point raised by @jakelangham in issue #129)
(addresses last of @jakelangham's points in issue #129 )
@jakelangham IMHO I have addressed the issues you raised in the current version of PR #139, see here https://discotec--139.org.readthedocs.build/en/139/ (minus the "advection solver", commented above). Let me know in case there is more open for you! (also to @EmilyBourne of course!) |
Hi @freifrauvonbleifrei - thanks for this. I think you really made some great improvements here. I've read through and overall things are much clearer. In particular the considerable work on the API is greatly appreciated. Only 2 tiny comments:
|
Regarding openjournals/joss-reviews#7018 - " Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?"
I cannot find documentation beyond the READMEs:
The webpage https://sparsegrids.org/ is mentioned in the "About" section but does not seem to mention DisCoTec:
Is there API documentation describing the available classes/methods somewhere? Perhaps something generated with doxygen?
The text was updated successfully, but these errors were encountered: