Snake case consistency #242
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hughcars
reviewed
May 6, 2024
| idx, Timer::Duration(Timer::Now() - t0).count()); | ||
|
|
||
| // Form and solve the linear system for a prescribed current on the specified source. | ||
| Mpi::Print("\n"); | ||
| A[step].SetSize(RHS.Size()); | ||
| A[step].UseDevice(true); | ||
| A[step] = 0.0; | ||
| curlcurlop.GetExcitationVector(idx, RHS); | ||
| curlcurl_op.GetExcitationVector(idx, RHS); |
There was a problem hiding this comment.
I could go either way on this really but curlcurl seemed OK as a variable name. The big gain is separating op from the core subject.
Comment on lines
+572
to
+573
| S_ij *= std::exp(1i * src_data.kn0 * src_data.d_offset); | ||
| S_ij *= std::exp(1i * data.kn0 * data.d_offset); |
There was a problem hiding this comment.
So the convention (which I agree with), is not snake once within a subscript _. Namely, not S_i_j.
There was a problem hiding this comment.
Yeah, double subscripts make more sense to me, S_i_j reads S-sub-i-sub-j which is different than S_ij. S_ij is closer to how you'd actually write it in tex.
sebastiangrimberg
force-pushed
the
sjg/bdr-postpro-dev
branch
from
650aae4 to
ce06cfa
Compare
sebastiangrimberg
force-pushed
the
sjg/snake-case-dev
branch
from
f3eab2e to
f4998ea
Compare
hughcars
approved these changes
May 8, 2024
sebastiangrimberg
force-pushed
the
sjg/bdr-postpro-dev
branch
from
69ef14a to
fb91f0f
Compare
sebastiangrimberg
force-pushed
the
sjg/snake-case-dev
branch
from
f4998ea to
e285f87
Compare
sebastiangrimberg
force-pushed
the
sjg/snake-case-dev
branch
from
e285f87 to
9903742
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have slowly moved towards consistent use of snake case across all variable naming, especially in the higher level classes and functions where variable names are longer and more descriptive. Though not explicitly enforced in the developer documentation or in PR reviews, this PR goes through some old code and updates variable names to be match the guidance.
The same change is also made for some output variables, namely the field names in the ParaView visualization files.
One name left unchanged is
iodata, which could/should be renamed toio_data. I didn't have particular feelings either way, but if changing it is preferable I can make that change here as well.Note: Based on #240