1. The Enzo Project
  2. Untitled project
  3. enzo-dev
  4. Pull requests

Pull requests

#273 Merged at e73ae9a
Repository
enzo-2.x-larrue
Branch
week-of-code
Repository
enzo-dev
Branch
week-of-code

Moved the order of the for-loops so that 'k' is the outer loop and 'i' is the inner loop (column-major order).

Author
  1. James Larrue
Reviewers
Description

I made this change in my code after some performance analysis.

Comments (9)

    1. James Larrue author

      The original file uses a mix of tabs and spaces for indentation. I have not be able to find any documentation on indentation style for Enzo, so I use two spaces for an indent. That is what seems the most common practice in the existing Enzo code. In this file, I only touched the lines in this section, so all the other lines are using the tab/space combination that they had before.

      I can go through the rest of the file and make the indentation consistent, using whatever indentation style people like. The only down-side is that a lot of lines will change, merely because the indentation has been made consistent. (Not a big down-side, just more work for reviewers.)

      1. Nathan Goldbaum

        I think it makes sense to maintain tab/spaces in files that already have it. I realize it's a bit of a lost cause. I generally use 4 spaces for tabs since it aligns with bitbucket's default (which also makes it easier for reviewers).

        1. James Larrue author

          After playing around with my editor's interpretation of the tab character, I managed to understand the indentation style of the original file.

          Do you like this better?

  1. Sam Skillman

    I think this sneaked in without the testing while the testing bot was down for a few days. I believe this may have introduced a FP roundoff change due to the summation order, which likely just requires a new gold standard, but it may be causing all the current PRs to fail. If someone could run the tests on this changeset and the one before it got merged to verify, that'd be great! Then I'll set up a new gold once verified.