Wojtek's work

[schmie]

Reported by hywel on 15 Sep 2011 15:08 UTC
This patch is the second half of Wojtek's work from his time in the CCS. It mainly concerns allowing us to take pressure boundaries from file (instead of just a sinusoid), and has been written in a way to make future expansion easier.

This patch is not the sum of his work. It's the diff between his last commit and the last common ancestral changeset with the trunk version on Entropy.

NB attached patch omits white-space differences

[schmie]

Modified by hywel on 19 Sep 2011 11:56 UTC

[schmie]

Comment by miguel on 19 Sep 2011 17:49 UTC
I'm mostly OK with the diff, just a few minor points:

• Code/SimulationMaster.cc: there are a couple of arguments not indented properly
• Code/lb/boundaries/BoundaryValues.cc: it looks like we allow public member variables (iolets[ioletIDs[i]]->density = ...). Are we OK with this or better Get/Set methods?
• Code/lb/boundaries/BoundaryValues.h: some of the constructor arguments don't use the i/o/b followed by capital convention. Similarly some member variables don't start with m.
• Code/lb/boundaries/iolets/*: several methods where density_cycle is an argument and same issue as previous bullet point.
• Some variables in Code/util/utilityFunctions.h could have more meaningful names than a, b, c, s, xs, etc
• Some indentation in Code/vis/Viewpoint.cc

[schmie]

Comment by hywel on 20 Sep 2011 13:12 UTC

• Did the reformatting.
• Public member vars not an issue as they're actually below the "private:" specifier in the header file.
• I think we're dropping this convention in favour of lowerCamelCase (most people use IDEs where it's easy to find a variables declaration and determine its scope).
• Renamed all occurrences to "densityCycle".
• New names: xVector, yVector, targetX, lowerIndex (in !LinearInterpolate), xLower, yLower, xHigher, yHigher, xAccuracy, yAccuracy, xBoundNew, xBoundOld, xSolution, ySolution (in Brent)
• I've checked it still builds.

All sound OK? Would you like to see another patch?

[schmie]

Comment by miguel on 20 Sep 2011 16:49 UTC
Yep, that sounds great. No need to upload another patch.

[schmie]

Comment by hywel on 21 Sep 2011 17:03 UTC
Pushed as changeset 9d5e50487f45