Use tool to style the SS codebase?

[k-doering-NOAA]

For r4ss, @iantaylor-NOAA realized at some point that it would be nice for the package to have consistent syle to make the codebase easier to read (For more info, see https://github.com/r4ss/r4ss/issues/399). He was able to use a tool called styler to automatically style the whole codebase and commit the changes, instead of manually going through and trying to clean up style (which takes a massive amount of time).

I was curious if something similar existed for C++ code and found the ClangFormat tool. Would there be interest in exploring using this tool to use consistent style within the SS codebase (if this tool can work for the tpl files, which I'm not totally sure about)?

I'm hoping this is a function we could run one time on the whole code base and commit the changes to improve style. I don't think this is high priority, but consistent style could improve readability of the code.

[Rick-Methot-NOAA]

I think this worth a try. the tpl code looks just like C##, so tool should work on it.

[iantaylor-NOAA]

I think this is a good idea.

Here's a suggested sequence of events, based on doing something similar with r4ss. I'm sure there's recommended best practices for how to do this somewhere out there, but I haven't come across them.

1. • [ ] test the formatting tool on a single TPL and adjust the settings as needed until the results look good. The ClangFormat tool seems to support a large set of style guide options described at https://clang.llvm.org/docs/ClangFormatStyleOptions.html and I wouldn't know where to start in choosing among them, but you could try applying each one and looking at the results to see what is the most readable or easiest to adjust to
2. • [ ] clean up any branches that are close to completion by wrapping up and merging any easy-to-finish changes
3. • [ ] apply the formatting to the full set of TPL files and confirm that the model still compiles and produces the same results (by pushing to a new branch and running all the tests)
4. • [ ] if things still look good, merge the formatting changes into the main branch
5. • [ ] either apply the formatting to all other branches so that the differences between branch and main are only those that matter for the branch (it might also work to rebase the branches onto main, but I suspect that the changes would be extensive enough that this wouldn't work)
6. • [ ] try to use the chosen style when writing new code, but acknowledge that we're only human, and won't always get it right
7. • [ ] occasionally re-run the reformatting tool to bring recent changes to the code into the chosen style

[Rick-Methot-NOAA]

There are a lot of settings in that style guide. Most seem related to indent and {} styles, so shold never break the code and only make it a bit more readable. A good start would be a consistent approach to {}; e.g. if{ code;} vs. if { code; }

or some other permutation, all of which can be seen somewhere in the SS code.

Richard D. Methot Jr. Ph.D.NOAA Fisheries Senior Scientist for Stock Assessments

Mobile: 301-787-0241

On Fri, Apr 16, 2021 at 12:43 PM Ian Taylor @.***> wrote:

I think this is a good idea.

Here's a suggested sequence of events, based on doing something similar with r4ss. I'm sure there's recommended best practices for how to do this somewhere out there, but I haven't come across them.

1.

• test the formatting tool on a single TPL and adjust the settings as needed until the results look good. The ClangFormat tool seems to support a large set of style guide options described at https://clang.llvm.org/docs/ClangFormatStyleOptions.html and I wouldn't know where to start in choosing among them, but you could try applying each one and looking at the results to see what is the most readable or easiest to adjust to 2.
• clean up any branches that are close to completion by wrapping up and merging any easy-to-finish changes 3.
• apply the formatting to the full set of TPL files and confirm that the model still compiles and produces the same results (by pushing to a new branch and running all the tests) 4.
• if things still look good, merge the formatting changes into the main branch 5.
• either apply the formatting to all other branches so that the differences between branch and main are only those that matter for the branch (it might also work to rebase the branches onto main, but I suspect that the changes would be extensive enough that this wouldn't work) 6.
• try to use the chosen style when writing new code, but acknowledge that we're only human, and won't always get it right 7.
• occasionally re-run the reformatting tool to bring recent changes to the code into the chosen style

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/163#issuecomment-821521769, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPV4IHRUEWKJW7XTJI72NTTJCHNPANCNFSM43B37UAA .

[k-doering-NOAA]

It is so easy to use all three ways of writing an if statement! It looks like you can choose a style guide to use for this function: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#configurable-format-style-options so we don't have to specify our own custom options. Choosing custom options seems like too much effort to me when other people have already done the work to put together a consistent style guide!

@Rick-Methot-NOAA , since you are the primary person making changes, perhaps you could choose one that you like the most and we could use it for the tool, then try to stick with it (like Ian said, as well as we can, but not perfectly) from here on out?

[Rick-Methot-NOAA]

exactly. no need for a custom approach here. just pick one. I'm not putting a high priority on this myself, but would be glad to see it get a trial. Ian's checklist was just right.

On Fri, Apr 16, 2021 at 3:28 PM Kathryn Doering @.***> wrote:

It is so easy to use all three ways of writing an if statement! It looks like you can choose a style guide to use for this function: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#configurable-format-style-options so we don't have to specify our own custom options. Choosing custom options seems like too much effort to me when other people have already done the work to put together a consistent style guide!

@Rick-Methot-NOAA https://github.com/Rick-Methot-NOAA , since you are the primary person making changes, perhaps you could choose one that you like the most and we could use it for the tool, then try to stick with it (like Ian said, as well as we can, but not perfectly) from here on out?

— 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/163#issuecomment-821634406, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPV4IAUR4BVJATWDOLLFQDTJC2Y3ANCNFSM43B37UAA .

[k-doering-NOAA]

I added the "wait" label, since there is a lot of shuffling of code lately in order to get an SS release version. I think we can explore this shortly after the release, since development is likely to lull around that time.

[nschindler-noaa]

I finally got the clang-format tool and it is the one I think we should use. It will integrate with GitHub (so it says). I have two files attached: the first with no options (clang-format SS_proced.tpl >> SS_proced-plain.tpl) and the second with a slightly modified style (clang-format -style="{BasedOnStyle: Webkit, IndentWidth: 2}" SS_proced.tpl >> SS_proced-webkit2.tpl). To modify the files in place rather than create new like I did, we would use -i before the style designation.

SS_proced-webkit2.tpl.txt SS_proced-plain.tpl.txt

[nschindler-noaa]

Using the command "clang-format -style="{BasedOnStyle: Webkit, IndentWidth: 2, BreakBeforeBraces: Allman}" SS_proced.tpl >> SS_proced-webkit3.tpl" produces the file attached. It has a return before all braces so the if statements look like if () { } else { } This will keep it all consistent. the other option was sometimes not compressing to "if () {" when there was a comment after the if statement. SS_proced-webkit3.tpl.txt

[iantaylor-NOAA]

@nschindler-noaa, thanks for working on this. I just looked at the three files you've shared and they all seem like a big improvement in readability with the addition of all the spaces and consistent indenting. I personally like the line breaks in the comments that you get with plain vs. the two webkit versions. That is, I like

  //  SS_Label_Info_7.3 #get Hrate from the parameter vector F_rate
  //  note that in SS_global  BETWEEN_PHASES is where F_rate, which is the
  //  parameter, gets assigned a starting value Hrate from Hrate calculated by
  //  hybrid in previous PH

better than

  //  SS_Label_Info_7.3 #get Hrate from the parameter vector F_rate
  //  note that in SS_global  BETWEEN_PHASES is where F_rate, which is the parameter, gets assigned a starting value Hrate from Hrate calculated by hybrid in previous PH

But perhaps there are areas where that would mess things up (like in the // SS_Label_file stuff at the top).

I also like the matching brackets associated with the webkit3 version.

Ultimately, I would defer to @Rick-Methot-NOAA and suggest we choose whichever combination of options he likes best, as that will make the changes least disruptive and easiest to adopt in future development. With r4ss, I've found that it's nicer to try to follow the style to some extent so that the GitHub action of applying the styling has fewer things to fix and is easier to look through for issues (I don't think I've found any so far).

[k-doering-NOAA]

Thanks @nschindler-noaa! I'm happy with whichever consistent styling option works for the group.

Also, I can work with you to get this into a github action, once we know how to set up and run locally.

[nschindler-noaa]

I've discovered that this is not compatible with our build process, so the search continues. I will be testing various configurations to see if one will work.

[nschindler-noaa]

Since ADMB treats the first two columns in a special way, clang-format doesn't work on tpl files. I haven't found a setting that works to leave them as is.

[iantaylor-NOAA]

@johnoel and @arni-magnusson, have either of you ever used clang-format or any similar tool to automatically style ADMB code?

We're looking for something which can be run in a GitHub Action to style the very large (and sometimes inconsistently formatted) Stock Synthesis code base. However, as @nschindler-noaa pointed out in the comment above, https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/163#issuecomment-1093192003, the special treatment of the first two columns in ADMB is causing problems.

[k-doering-NOAA]

Since ADMB treats the first two columns in a special way, clang-format doesn't work on tpl files. I haven't found a setting that works to leave them as is.

Neal, what is the "special way" ADMB is treating the tpl files compared to standard C++ code? I feel like I need a little more detail to understand the issue.

Thanks for all your work on this! If nothing else, it can probably pay off for FIMS, which still needs an automatic c++ code styling tool, I think.

[iantaylor-NOAA]

I'm assuming that the issue is related to the code needing to always be indented 2 spaces with the exception of the template sections which need to have no indent, as shown in the figure below from https://www.admb-project.org/developers/workshops/la-jolla-2010/ADMBGettingStartedGuide.pdf Maybe we can somehow train clang-format to know what the template section headers are. All of them are listed at https://github.com/admb-project/admb/blob/main/contrib/emacs/admb.el#L154-L158 (and surely other places).

@nschindler-noaa, let me know if you're seeing other issues.

[k-doering-NOAA]

Ah, I see, that makes sense. Hmm, if we can't find an option to do this in the clang-style tool, I wonder if we could find some other tools that is more flexible or build a custom tool to use after that adds the two spaces back in (for instance, we could run clang-style, then write R or other language code to add the 2 spaces back in where needed).

[Rick-Methot-NOAA]

This could be a good reason to finally leave tpl behind. There is no reason that we could not create the ss.cpp and ss.htp files once, then do all future development in that fully c++ environment.

[iantaylor-NOAA]

Good point, Rick. Would it be possible to split up the cpp and htp files in some logical way (without a huge amount of restructuring) to avoid all edits taking place in the same giant file?

[Rick-Methot-NOAA]

I am sure we could do that, but it would take some work. @nschindler-noaa Could you prepare a proposed restructuring?

[allanhicks]

I did this for earlier versions of Awatea (a branch of Coleraine) and for a Black Oreo assessment model in NZ. I wrote initial TPL code, used tpl2cpp to create the c++ files, and then worked on the C++ code. It allowed me to specifically use the object oriented structure of C++ and organize the code using classes and functions. It was partly successful, but I still had to fit into the ADMB paradigm. Here are some quick thoughts on pros and cons that I experienced:

PROS

• could leverage the organizational benefits of C++, with separate files for classes
• used TEMPLATE to make functions variables with derivatives or variables without derivatives (to speed up simulations)
• did not have to worry about specifics of TPL files

CONS

• had to maintain the class structure created by ADMB (tpl2cpp), which requires a little bit of careful accounting
• could not figure out how to completely separate some classes from ADMB classes
• Was difficult to collaborate with users that wanted to edit code but did not know C++ as well as ADMB
• Difficult to update to newer versions of ADMB because I would have to recreate the tpl and then run tpl2cpp again.

In the end, I ended up creating a tpl from the C++ code so that I could collaborate with others and allow them to edit the code and update to newer versions of ADMB. I don't believe that Awatea is used any more.

I wonder if a different solution would be to maintain use of a tpl file, but call in C++ classes and files. I created an example of how to do this when I was working with a programmer a few years ago. Steve Martell also explained an approach that was similar. I could dig up that example and discuss if desired.

Possibly the best way to avoid the tpl file would be to use autodiff functions directly in C++, but I was never able to accomplish this. CASAL uses their own autodiff library (betadiff I believe) and developers modify code directly in C++ classes. I'm not sure if this an option, but it could be time consuming.

I hope that this is helpful and follows along what the thoughts of this thread are. I'm sure that the smarter people involved in this project could solve the issues that I was having. I'm happy to discuss my experiences and share examples if desired.

[Rick-Methot-NOAA]

Thanks a ton Allan for this info and background. I don't want to make this too big of a project as our real effort at improved coding and modularization is with FIMS.

[jimianelli]

Just to note, and maybe remind folks, that ADMB is built on top of the Autodif library. Autodif was the first product (multifanCL is written in that) so maybe that's a helpful way to think about it.

In trying to find that old documentation, came across this https://autodiff.github.io, interesting! maybe FIMS is on to these options but maybe irrelevant given what's already available in TMB.

https://www.admb-project.org/docs/manuals/ and specific to autodif here https://github.com/admb-project/admb/releases/download/admb-12.3/autodif-12.3.pdf .

On Mon, Apr 11, 2022 at 11:37 AM Allan Hicks @.***> wrote:

I did this for earlier versions of Awatea (a branch of Coleraine) and for a Black Oreo assessment model in NZ. I wrote initial TPL code, used tpl2cpp to create the c++ files, and then worked on the C++ code. It allowed me to specifically use the object oriented structure of C++ and organize the code using classes and functions. It was partly successful, but I still had to fit into the ADMB paradigm. Here are some quick thoughts on pros and cons that I experienced:

PROS

• could leverage the organizational benefits of C++, with separate files for classes
• used TEMPLATE to make functions variables with derivatives or variables without derivatives (to speed up simulations)
• did not have to worry about specifics of TPL files

CONS

• had to maintain the class structure created by ADMB (tpl2cpp), which requires a little bit of careful accounting
• could not figure out how to completely separate some classes from ADMB classes
• Was difficult to collaborate with users that wanted to edit code but did not know C++ as well as ADMB
• Difficult to update to newer versions of ADMB because I would have to recreate the tpl and then run tpl2cpp again.

In the end, I ended up creating a tpl from the C++ code so that I could collaborate with others and allow them to edit the code and update to newer versions of ADMB. I don't believe that Awatea is used any more.

I wonder if a different solution would be to maintain use of a tpl file, but call in C++ classes and files. I created an example of how to do this when I was working with a programmer a few years ago. Steve Martell also explained an approach that was similar. I could dig up that example and discuss if desired.

Possibly the best way to avoid the tpl file would be to use autodiff functions directly in C++, but I was never able to accomplish this. CASAL uses their own autodiff library (betadiff I believe) and developers modify code directly in C++ classes. I'm not sure if this an option, but it could be time consuming.

I hope that this is helpful and follows along what the thoughts of this thread are. I'm sure that the smarter people involved in this project could solve the issues that I was having. I'm happy to discuss my experiences and share examples if desired.

— Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/163#issuecomment-1095415536, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAUW7YXEOLXF22UBEXHHZDDVERWPHANCNFSM43B37UAA . You are receiving this because you are subscribed to this thread.Message ID: @.*** com>

-- Jim Ianelli

[Rick-Methot-NOAA]

Thanks Jim. This autodif package seems different from the original package behind ADMB, or am I confused by the author of that github.io site? Let's assure awareness in FIMS. Also, in talking to Thorson yesterday, I learned that TMB has a new custom autodiff backend that is even better at sparseness detection.

[jimianelli]

sorry, the github.io one is different, and began in 2018...not 1988.

On Tue, Apr 12, 2022 at 11:54 AM Richard Methot @.***> wrote:

Thanks Jim. This autodif package seems different from the original package behind ADMB, or am I confused by the author of that github.io site? Let's assure awareness in FIMS. Also, in talking to Thorson yesterday, I learned that TMB has a new custom autodiff backend that is even better at sparseness detection.

— Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/163#issuecomment-1097099638, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAUW7YSQQ556OW52MDPCHQLVEXBGLANCNFSM43B37UAA . You are receiving this because you commented.Message ID: @.***>

-- Jim Ianelli

[nschindler-noaa]

I have created a tpl-format.exe that cleans up after clang-format. I have just pushed to the issue 163 branch changes to all the SS files except the SS_param.tpl, SS_read, and SS_versioninfo files. These did not benefit from the process.

The process: run the batch file "pretty_tpl.bat" followed by a filename (we can discuss my reasons for making it file-by-file if you wish). Inside pretty_tpl.bat, it first calls clang-format (a .clang-format file has been added to the repository to regulate that behaviour), then call tpl-format.exe (a new utility in a new repository) that fixes most of the problems clang-format creates for tpl code (as I say above, those files, when run, cause the build to crash - if someone can figure out why, I'd like to know).

This does requires clang-format and tpl-format to be in the path or in the source folder.

[Rick-Methot-NOAA]

great to learn of the progress. will take a look soon. First guess on the build crash is the single space before LOCAL_CALCS and END_CALCS

[nschindler-noaa]

Got that, but it may be related since it happens right after on an init_ statement.

[nschindler-noaa]

Thanks to all for you input, especially Ian for pointing me to the ADMB info.

[k-doering-NOAA]

Wow, great work, Neal! It's awesome to see you found a solution for the TPL formatting issues and thanks for explicitly outlining the steps to style - I have a few questions and comments:

1. Could you point me to the tpl-format.exe?
2. Why did a few files not benefit from the process? Did formatting them cause any harm?
3. Note that the branch is 23 commits behind main - I think this is why there will be merge conflicts if we were to try to merge to main (see https://github.com/nmfs-stock-synthesis/stock-synthesis/compare/main...163-use-tool-to-style-the-ss-codebase). I wonder if it would be possible to undo the file style changes, catch up the branch to main, and then rerun styling again on the files? I think this may give us cleaner style than trying to deal with the merge conflicts
4. This is the perfect kind of job to set up as a github action! I think it would be great to do this. If you would like to learn more about the process of setting up github actions, perhaps we can code it up together? Otherwise, I'm happy to work on it by myself, up to you!

[nschindler-noaa]

1. The repository (now with the executable) is at https://github.com/nmfs-stock-synthesis/tpl-format
2. Formatting those exceptions caused the build to fail. I couldn't figure out what was wrong, so omitted them.
3. I thought that might happen. we could delete the branch and start over, unless that deletes this conversation. I've had trouble trying to merge main back into a branch where there are differences in the same files.
4. With the addition of a few files, it seems pretty simple. Actually, since it is file-by-file, it could be run by someone who makes changes to a file and then wants to "prettify" it. Otherwise, a script could be written to do the code base (with exceptions).

3 (second thought). I'll try to work with the existing branch for a bit, but anticipate giving up after a few hours.

On Wed, Apr 13, 2022 at 5:52 AM Kathryn Doering @.***> wrote:

Wow, great work, Neal! It's awesome to see you found a solution for the TPL formatting issues and thanks for explicitly outlining the steps to style - I have a few questions and comments:

1. Could you point me to the tpl-format.exe?
2. Why did a few files not benefit from the process? Did formatting them cause any harm?
3. Note that the branch is 23 commits behind main - I think this is why there will be merge conflicts if we were to try to merge to main (see main...163-use-tool-to-style-the-ss-codebase https://github.com/nmfs-stock-synthesis/stock-synthesis/compare/main...163-use-tool-to-style-the-ss-codebase). I wonder if it would be possible to undo the file style changes, catch up the branch to main, and then rerun styling again on the files? I think this may give us cleaner style than trying to deal with the merge conflicts
4. This is the perfect kind of job to set up as a github action! I think it would be great to do this. If you would like to learn more about the process of setting up github actions, perhaps we can code it up together? Otherwise, I'm happy to work on it by myself, up to you!

[Rick-Methot-NOAA]

Can we make the code for extracting the ss_label info accessible in the same location as this code for reformatting?

[nschindler-noaa]

Kathryn created a second branch with updated files. When I try to reformat and build, I get the following error: Error: Unable to parse "ss.tpl". Error -- REPORT_SECTION must be defined before FINAL_SECTION

Error: Unable to build.

Has anyone seen this error before? I checked the ss.tpl and it is true that the FINAL_SECTION is defined before REPORT_SECTION, but it works on unmodified files.

[Rick-Methot-NOAA]

I presume that is an error thrown during the tpl2cpp stage. I have never seen it.

In the unstylized ss.cpp file final: void model_parameters::final_calcs() { } is before: void model_parameters::report(const dvector& gradients) { }

[nschindler-noaa]

On branch 163-new-style, I've added the files .clang-format and pretty_tpl.bat. tpl-format.exe has been updated and is available on github at https://github.com/nmfs-stock-synthesis/tpl-format Updated all tpl files except SS_readstarter.tpl, SS_readcontrol_330.tpl, SS_readdata_330.tpl. SS_param.tpl is changed slightly at the beginning (edited by hand) but still compiles.

On Thu, Apr 14, 2022 at 10:21 AM Neal Schindler - NOAA Affiliate < @.***> wrote:

The first problem I've found is in the FUNCTION statement. clang-format inserts a space before the parentheses. I've updated tpl-format for this and as I check other files, I will update if needed. FIrst file: SS_ALK.tpl.

On Wed, Apr 13, 2022 at 4:30 PM Richard Methot @.***> wrote:

I presume that is an error thrown during the tpl2cpp stage. I have never seen it.

In the unstylized ss.cpp file final: void model_parameters::final_calcs() { } is before: void model_parameters::report(const dvector& gradients) { }

— Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/163#issuecomment-1098572828, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARUZACXFLJYB3C7VGMYEHRLVE5KIJANCNFSM43B37UAA . You are receiving this because you were mentioned.Message ID: @.***>

--

Neal Schindler

Programmer Analyst, Caelum

NWFSC (206) 860-3407

cell (206) 909-0505

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

=(((`<

--

Neal Schindler

Programmer Analyst, Caelum

NWFSC (206) 860-3407

cell (206) 909-0505

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

=(((`<

[k-doering-NOAA]

Awesome, I'm glad you were able to find a solution @nschindler-noaa ! Thanks for all of your time and effort on this.

Also, I don't know if we added in context on this thread about the github action, but Neal let me know that he doesn't think we are ready to automatically style the codebase on every commit, as there are still some fringe cases where the conversion may fail (because we are styling a tpl and not C++ code). Perhaps we could work towards this goal, though. It seems reasonable to me to perhaps use the process Neal came up with to restyle after every release, perhaps refining it a bit each time?

[Rick-Methot-NOAA]

I look forward to looking at the styled files. Do they exist or do I need to create by using the tools?

I think it makes sense to run styler once as we get ready to do a release. As we all learn the style, we can simply use it as we create and modify code.

[Rick-Methot-NOAA]

I switched to the styled branch and compiled it locally.
I looked over some of the code and it all seems very clean and neat and consistent - as intended. Good work Neal. Two questions:

• [ ] What should we do with branches that have not yet been styled; seems we will have git conflicts?
• [ ] when should we switch to the styled code as the new main?

[k-doering-NOAA]

What should we do with branches that have not yet been styled; seems we will have git conflicts?

Yes, that may be the case; I think after we merge the style branch into main, the main branch can be merged into any feature branches (I don't think there are that many) and conflicts can be dealt with on the branches.

when should we switch to the styled code as the new main?

I think pretty soon is a good time, since there is not a lot of active development. Maybe Neal can submit a pull request to merge the 163-new-style into main, and @Rick-Methot-NOAA can merge it in if all seems ready to go (It sounds like Rick has already tested it)?

[Rick-Methot-NOAA]

Good plan. Exactly what I was thinking as the path forward. Let's be sure first that 163-new-style has all the fixes just released in 3.30.19.01.

[k-doering-NOAA]

Good plan. Exactly what I was thinking as the path forward. Let's be sure first that 163-new-style has all the fixes just released in 3.30.19.01.

Yes, it does!

[Rick-Methot-NOAA]

Well, the merge conflicts were huge. @nschindler-noaa next week can you run the style tools on the transitional SPR branch, then merge the styled main onto that branch so I can then continue work on transitional SPR?

[Rick-Methot-NOAA]

thanks Neal for getting transitional_SPR converted.

@nschindler-noaa I just noticed that there is one type of white space that seems unnecessary and inconvenient. There is a space before the "(" in statements like: fleet_type (f). Is that according to a convention that I do not know? Seems better to me as: fleet_type(f) but I am willing to go with the convention if that is the consensus

[nschindler-noaa]

That is a function of clang-format. There is a setting in the file .clang-format "SpaceBeforeParens: ControlStatements" which (as is) will put a space before the first parenthesis in control statements (if, for, etc.). However, I did change it to "Always" for a couple files to see what would happen and I failed to back that change out. clang-format can be run for those files affected and it will reverse that.

On Tue, Apr 26, 2022 at 9:32 AM Richard Methot @.***> wrote:

thanks Neal for getting transitional_SPR converted.

@nschindler-noaa https://github.com/nschindler-noaa I just noticed that there is one type of white space that seems unnecessary and inconvenient. There is a space before the "(" in statements like: fleet_type (f). Is that according to a convention that I do not know? Seems better to me as: fleet_type(f) but I am willing to go with the convention if that is the consensus

— Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/163#issuecomment-1110006520, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARUZACX4XFUXLASPFWB2PLLVHALAHANCNFSM43B37UAA . You are receiving this because you were mentioned.Message ID: @.***>

--

Neal Schindler

Programmer Analyst, Caelum

NWFSC (206) 860-3407

cell (206) 909-0505

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

=(((`<

[nschindler-noaa]

Remember to use the pretty-tpl.bat to reformat.

[Rick-Methot-NOAA]

Neal, please run pretty-tpl.bat on main and on transitional_SPR with settings that retain space here: if (...) and remove space here: fleet_type(f)

[nschindler-noaa]

I have pushed up changes to the trans SPR branch and also created a new branch, 163-make-whitespace-consistent, and created a pull request which is going through checks right now.

On Tue, Apr 26, 2022 at 11:02 AM Richard Methot @.***> wrote:

Neal, please run pretty-tpl.bat on main and on transitional_SPR with settings that retain space here: if (...) and remove space here: fleet_type(f)

— Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/163#issuecomment-1110095184, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARUZACUWPXJMXES2LTUV6X3VHAVTVANCNFSM43B37UAA . You are receiving this because you were mentioned.Message ID: @.***>

--

Neal Schindler

Programmer Analyst, Caelum

NWFSC (206) 860-3407

cell (206) 909-0505

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

=(((`<

[k-doering-NOAA]

Closing this issue, as @nschindler-noaa is successfully using clang-format. I think discussion on the style guide can continue on #311 , and other issues could be opened to work on specific problems that still exist for getting clang format to work automatically!

[nschindler-noaa]

The first problem I've found is in the FUNCTION statement. clang-format inserts a space before the parentheses. I've updated tpl-format for this and as I check other files, I will update if needed. FIrst file: SS_ALK.tpl.

On Wed, Apr 13, 2022 at 4:30 PM Richard Methot @.***> wrote:

I presume that is an error thrown during the tpl2cpp stage. I have never seen it.

In the unstylized ss.cpp file final: void model_parameters::final_calcs() { } is before: void model_parameters::report(const dvector& gradients) { }

— Reply to this email directly, view it on GitHub https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/163#issuecomment-1098572828, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARUZACXFLJYB3C7VGMYEHRLVE5KIJANCNFSM43B37UAA . You are receiving this because you were mentioned.Message ID: @.***>

--

Neal Schindler

Programmer Analyst, Caelum

NWFSC (206) 860-3407

cell (206) 909-0505

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

=(((`<