ss3-source-code icon indicating copy to clipboard operation
ss3-source-code copied to clipboard

code documentation

Open Rick-Methot-NOAA opened this issue 3 years ago • 12 comments

create working branch "cleanup_and_document_code_filelist"

work on editing and updating SS_filelist.txt I will convert filelist to a markdown file before proceeding much further. This file is a very high level overview, so not nearly as detailed as the SS_codeflow that Neal is working on.

Also will remove some code deadwood and add more SS_label entries in code as I proceed.

Rick-Methot-NOAA avatar May 10 '21 22:05 Rick-Methot-NOAA

Good idea, Rick! Markdown is much more readable.

Just in case it is useful, I often refer to this github markdown cheatsheet as a reminder of the formatting options.

Another tip is you can edit files directly on github and therefore have an easy preview of how the markdown looks if the file ends in .md - the downside is only 1 file can be commited at a time.

k-doering-NOAA avatar May 10 '21 22:05 k-doering-NOAA

I turned this into a major project and think I have succeeded. My intention was to easily create a brief summary of all the ss code to complement the extensive documentation that Neal has extracted and augmented in SS Flow Outline.

What I did was extend the approach that Neal is using (extracting every line with "SS_Label"). All of the SS tpl files now have 2-10 new lines at the top of the file. These lines briefly describe the contents of that file, including the names of any functions in that file. Each of these lines begins with "SS_Label_file #" after the hash character is text, which includes markdown formatting codes.

The next step is to extract from ss.tpl, which has all the tpl files appended to each other, all the lines with the SS_Label_file text, put that extraction into a file. Then remove all the "SS_Label_file " to retain just the information and markdown code. Results are in the attached file. labels.md

Now feeling that whenever we are working in a section of the code, the SS_Label comments should be reviewed and improved. @nschindler-noaa @k-doering-NOAA @iantaylor-NOAA

Rick-Methot-NOAA avatar May 12 '21 01:05 Rick-Methot-NOAA

It seems like a great idea to improve documentation as code is reviewed!

It just occured to me that we could potentially use an already existing documentation system, like doxygen, as we are essentially creating our own system at this point! For example, the admb documentation is using doxygen. It works pretty similar to Roxygen and to what Neal and Rick have been doing with SS. Info about files/functions is added in comments with tags throughout the code, e.g.,:

https://github.com/admb-project/admb/blob/f5bbd3f45cb8bdbe103bb87fe9366a6d72059ec7/src/nh99/xxmcmc.cpp#L91-L125

Which can be generated by doxygen to: http://api.admb-project.org/classfunction__minimizer.html#a1b46b0d9381cb3f6775e53aa84a1b7ad

It looks like doxygen also does a good job of tying together functions, files names and descriptions, and the source code.

I'm sure there are other options, too. I think d oxygen would work for SS b/c it works for C++ code...

Of course, changing to a formal documentation system would require time, but like Rick said, it looks like code documentation has become a major project, and maybe it would be worth using a system that already exists rather than invent our own.

k-doering-NOAA avatar May 12 '21 15:05 k-doering-NOAA

Definitely worth checking into doxygen. I've known about it with ADMB but had not considered trying it for SS because the SS code is "messy". Neal/Kathryn - can one of you look into the feasibility of doxygen working with the current SS_Label system?

Rick

On Wed, May 12, 2021 at 8:12 AM Kathryn Doering @.***> wrote:

It seems like a great idea to improve documentation as code is reviewed!

It just occured to me that we could potentially use an already existing documentation system, like doxygen https://www.doxygen.nl/index.html, as we are essentially creating our own system at this point! For example, the admb documentation http://api.admb-project.org/index.html is using doxygen. It works pretty similar to Roxygen and to what Neal and Rick have been doing with SS. Info about files/functions is added in comments with tags throughout the code, e.g.,:

https://github.com/admb-project/admb/blob/f5bbd3f45cb8bdbe103bb87fe9366a6d72059ec7/src/nh99/xxmcmc.cpp#L91-L125

Which can be generated by doxygen to:

http://api.admb-project.org/classfunction__minimizer.html#a1b46b0d9381cb3f6775e53aa84a1b7ad

It looks like doxygen also does a good job of tying together functions, files names and descriptions, and the source code.

I'm sure there are other options, too. I think d oxygen would work for SS b/c it works for C++ code...

Of course, changing to a formal documentation system would require time, but like Rick said, it looks like code documentation has become a major project, and maybe it would be worth using a system that already exists rather than invent our own.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/176#issuecomment-839853139, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPV4IAJVXLSAU7RBTGKQ6LTNKLE3ANCNFSM44SVKGUQ .

Rick-Methot-NOAA avatar May 12 '21 15:05 Rick-Methot-NOAA

I think we would have change to doxygen's way of doing things (I'm guessing this based on my experience with Roxygen, where you have to use specific tags to create the documentation, like @param to document a parameter), and I'm not sure if the use of globals would cause issues for the documentation system or not - my guess is "no". We might be able to leave in the SS_Label system, though, and also add in doxygen documentation.

@nschindler-noaa , would you be interested into looking into this more? I know this has been your main project recently. But I can help out if you would like, just let me know.

k-doering-NOAA avatar May 12 '21 15:05 k-doering-NOAA

I have used doxygen before and like it. It can produce web pages as well as a text document. You only have to document what you want. I'm not sure how program flow would work with it, though. I need to review how to use it. Seems like Roxygen is similar but has different syntax.

On Wed, May 12, 2021 at 8:32 AM Kathryn Doering @.***> wrote:

I think we would have change to doxygen's way of doing things (I'm guessing this based on my experience with Roxygen, where you have to use specific tags to create the documentation, like @param to document a parameter), and I'm not sure if the use of globals would cause issues for the documentation system or not - my guess is "no". We might be able to leave in the SS_Label system, though, and also add in doxygen documentation.

@nschindler-noaa https://github.com/nschindler-noaa , would you be interested into looking into this more? I know this has been your main project recently. But I can help out if you would like, just let me know.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/176#issuecomment-839869068, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARUZACWNFTV32GF6VG7BK5LTNKNQ5ANCNFSM44SVKGUQ .

--

Neal Schindler

Programmer Analyst, Caelum

NWFSC (206) 860-3407

cell (206) 909-0505

=(((< >=(((< >=(((`<

=(((`<

nschindler-noaa avatar May 12 '21 16:05 nschindler-noaa

Thanks Neal. Please look into this. Hopefully the first step is as simple as global search and replace for SS_Label with

On Wed, May 12, 2021 at 9:18 AM Neal Schindler @.***> wrote:

I have used doxygen before and like it. It can produce web pages as well as a text document. You only have to document what you want. I'm not sure how program flow would work with it, though. I need to review how to use it. Seems like Roxygen is similar but has different syntax.

On Wed, May 12, 2021 at 8:32 AM Kathryn Doering @.***> wrote:

I think we would have change to doxygen's way of doing things (I'm guessing this based on my experience with Roxygen, where you have to use specific tags to create the documentation, like @param to document a parameter), and I'm not sure if the use of globals would cause issues for the documentation system or not - my guess is "no". We might be able to leave in the SS_Label system, though, and also add in doxygen documentation.

@nschindler-noaa https://github.com/nschindler-noaa , would you be interested into looking into this more? I know this has been your main project recently. But I can help out if you would like, just let me know.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/176#issuecomment-839869068 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/ARUZACWNFTV32GF6VG7BK5LTNKNQ5ANCNFSM44SVKGUQ

.

--

Neal Schindler

Programmer Analyst, Caelum

NWFSC (206) 860-3407

cell (206) 909-0505

=(((< >=(((< >=(((`<

=(((`<

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/176#issuecomment-839912647, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPV4IAXU7QPBFNKI5VMGEDTNKS4NANCNFSM44SVKGUQ .

Rick-Methot-NOAA avatar May 12 '21 16:05 Rick-Methot-NOAA

There is a problem using doxygen with ADMB produced code: the comments tend to be inside functions which are ignored by doxygen. I tried several ideas (different formatting, different positions), using SS_ALK.tpl to test, and have had no success. So, we are left trying to document the tpl code. I haven't had much success there, either (don't know why). Trying to find a format that will work with doxygen and also allow ADMB to generate the code is a job. However, doxygen does read markdown files fine, so the .md files are parsed.

On Mon, May 17, 2021 at 10:28 AM Richard Methot @.***> wrote:

Assigned #176 https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/176 to @nschindler-noaa https://github.com/nschindler-noaa.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/176#event-4755963219, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARUZACRRYYTRBB6JPNIQRPTTOFG4RANCNFSM44SVKGUQ .

--

Neal Schindler

Programmer Analyst, Caelum

NWFSC (206) 860-3407

cell (206) 909-0505

=(((< >=(((< >=(((`<

=(((`<

nschindler-noaa avatar May 17 '21 23:05 nschindler-noaa

Thanks for looking into this, Neal - that is a shame to hear! Hmm, I wonder if using the EXTENSION_MAPPING configuration argument to map .tpl files to C++ (or some other C like lang) would work to allow docs to be created from the tpl files? Here's a page on configuration: https://www.doxygen.nl/manual/config.html#config_project

k-doering-NOAA avatar May 18 '21 15:05 k-doering-NOAA

I wrote to Johnoel to see if he has advice. Another possibility is to stop editing at the tpl level and instead switch to development in the ss.cpp and ss.htp files

On Tue, May 18, 2021 at 8:30 AM Kathryn Doering @.***> wrote:

Thanks for looking into this, Neal - that is a shame to hear! Hmm, I wonder if using the EXTENSION_MAPPING configuration argument to map .tpl files to C++ (or some other C like lang) would work to allow docs to be created from the tpl files? Here's a page on configuration: https://www.doxygen.nl/manual/config.html#config_project

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/176#issuecomment-843271869, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPV4IBGVNEI7J5ORBCZM3DTOKBYHANCNFSM44SVKGUQ .

Rick-Methot-NOAA avatar May 18 '21 15:05 Rick-Methot-NOAA

@nschindler-noaa

Hi Neal. Have we taken this as far as we can? Seems to me we (I) just need to be more diligent at adding the comments into the code.

Rick-Methot-NOAA avatar Nov 16 '22 23:11 Rick-Methot-NOAA

I think so. And yes, keep commenting.

nschindler-noaa avatar Nov 17 '22 20:11 nschindler-noaa