Open d7919 opened 6 years ago
Definitely agree that Mesh needs simplifying, and removing unused functions is a good start.
iterateBndryLowerOuterY
, iterateBndryLowerInnerY
, iterateBndryUpperOuterY
, iterateBndryUpperInnerY
-- I think these were for Nick's model, where he wanted to apply different boundary conditions to these separate regions. There must be a better way to specify different regions though. Perhaps this is possible with the index iterator regions?
lowPass_poloidal
, slice_r_y
, get_ri
, set_ri
(removing these would allow removal of cfft
in the fft routines) -- These were added by NFRI for their gyrofluid modelling. I don't think they use the github version of BOUT++, but even if they did these should be separate functions, not part of Mesh.
Switch_YZ
, Switch_XZ
-- Similar; if they should be here at all, then they should be standalone functions
readInts
-- I think this was for reading vectors of integers, in particular for branch cuts in a more general mesh decomposition (quilt mesh).
addBoundary
(unit tests) -- This was just added to the unit test FakeMesh (not real mesh) as a way to test the boundary conditions. Agree that there may be a more generic use, such as replacing the iterateBndryLowerOuterY functions?
sendToProc
, receiveFromProc
UpXSplitIndex
, DownXSplitIndex
sendYOutIndest
, sendYOutOutdest
, sendYInOutdest
, sendYInIndest
irecvYOutIndest
, irecvYOutOutdest
, irecvYInOutdest
, irecvYInIndest
These are too low-level, and constrain the way that Mesh can be implemented. I would like to see these replaced by a more generic/ higher-level function in terms of the mathematical function it is trying to implement.
smoothSeparatrix
-- This kind of thing is sometimes needed, but should be a separate function. There should be a way to get "special" indices, such as locations of separatrices, from the Mesh. If that could be done, then this could be a separate function.Yes I do wonder if a unified treatment of different regions might indeed be possible, if not with the current iterator then with the single index version at least. This could simplify some of this.
The addBoundary
routine does appear in main mesh
-- are you suggesting it shouldn't (as could just be a part of FakeMesh
as an extension)?
iterateBndryLowerOuterY
, iterateBndryLowerInnerY
, iterateBndryUpperOuterY
, iterateBndryUpperInnerY
These were put in to allow more flexibility iterating over boundaries when the branch cuts are specified. Agree that if a better, unified treatment is possible these could (should) be removed. I suspect the usage of these function is limited to just me currently though so if there is a push to remove them sooner rather than later I can work around.
I don't think there's any desperate need to remove any of these if they are being used/useful but by identifying those bits of the code that aren't used very much/in many places it can be a bit easier to think about refactoring etc.
An update on the current situation. The following are still in Mesh
:
addBoundary
readInts
smoothSeparatrix
sendToProc
, receiveFromProc
UpXSplitIndex
, DownXSplitIndex
sendYOutIndest
, sendYOutOutdest
, sendYInOutdest
, sendYInIndest
irecvYOutIndest
, irecvYOutOutdest
, irecvYInOutdest
, irecvYInIndest
The following can be replaced by SDI regions once #775 goes in:
iterateBndryLowerOuterY
iterateBndryLowerInnerY
iterateBndryUpperOuterY
iterateBndryUpperInnerY
Some years later, the following are still in Mesh
:
There are a reasonable number of functions in the
mesh
parent class that don't seem to be used anywhere inbout++
, the examples or the tests. Removing these and simplifying the parent class might make things a bit clearer so I'd be interested in deleting these and/or exporting any that don't need to be member functions to some other location.The list of what I think are unused functions is:
iterateBndryLowerOuterY
,iterateBndryLowerInnerY
,iterateBndryUpperOuterY
,iterateBndryUpperInnerY
lowPass_poloidal
,slice_r_y
,get_ri
,set_ri
(removing these would allow removal ofcfft
in the fft routines)Switch_YZ
,Switch_XZ
readInts
(~300 lines)
Some are just used in one place but may be more widely used in other's models (may be worth spinning off to new location in some cases?):
addBoundary
(unit tests)Following just for non-local example (but presumably used in some models?):
sendToProc
,receiveFromProc
UpXSplitIndex
,DownXSplitIndex
sendYOutIndest
,sendYOutOutdest
,sendYInOutdest
,sendYInIndest
irecvYOutIndest
,irecvYOutOutdest
,irecvYInOutdest
,irecvYInIndest
Following just for
dalf3
examplesmoothSeparatrix
In total I think this would remove somewhere in the region of 500 lines (~20% of boutmesh.cxx)