Do we want to keep USE_C_INDEX?
Issue #17
resolved
If this is true then we pass off index lookup to some provided c code. This leads to a fair bit of extra code in gs2_layouts.fpp and effectively duplicates the index conversion calculations, increasing the complexity of maintenance.
Does anyone use this and if so are there any benefits?
Comments (10)
-
-
reporter
The corresponding
agk_layouts.fpplooks similar to the gs2 version (with this as an option) from a while ago before we optimised out the case statements. -
From my profiling, removing the case statements is a big optimization, particularly at scale. This is probably something that should be ported to AGK. Is there any particular reason for
gs2_layoutsandagk_layoutsto be distinct anyway? It seems to me like the sort of file that could be inutils? -
I need to think a bit (and look back through emails) to be sure I understand the full story about the C indexing before we axe it. We definitely needed it at that time. As I work through this issue, could someone summarize the best possible reason or two for believing we no longer need it?
Second, with respect to the layouts questions: I think they should stay distinct for at least a short time. I will also work through the possible issues associated with merging them.
-
reporter
Optimisation work has removed the case statements in the pure fortran version of these routines -- I think performance was probably the motivation for the C code, but I think we now get this in the pure fortran version. The motivation for removing this is to aid maintenance (less code whilst maintaining features+capability) and reduce the places we make direct use of C object code. It also would help reduce the surface area that has to be tested (i.e. we currently will need to test both versions of the indexing routines rather than just one).
I'm not sure it makes sense to merge the two layouts modules any more as GS2 has developed new layouts, which would be of less use to AGK etc.
-
@daviddickinson So the pure Fortran version has good performance without the C links. Does this require the object oriented features within Fortran (type binding, or whatever it is called there)? I don't mind agreeing to supporting that level of sophistication (i.e., upgrading some systems here at Maryland) but I want to be sure I know what we are talking about.
Perl manufacturing? Sure, I agree.
Separate layouts for now? I agree. The AGK crowd doesn't need to be exposed to the GS2 stuff. If there is a way to handle that (common stuff in common, specialized layouts in a new module, for example) then the AGK folks would get improvements without the exposure and that would be good.
-
reporter
I don't think this requires type binding -- it was just an extension to existing use of the original layout types (which just contained data, rather than data and methods). We just shifted some work into the initialisation to simplify the index calculation calls by removing branching. I don't think anyone has timed the two different methods but looking at the two sets of code I think the Fortran code certainly doesn't have more operations to do (and I think there may be fewer).
-
Ok, sounds like good progress, then. I will email Tomo to see if he has any comments.
-
My memory is unreliable, but I vaguely recall that Tomo introduced this to gain performance. I don't recall using it though and how significant it was. The more recent improvements to the fortran helped.
-
- changed status to resolved
Resolved by PR #174
- Log in to comment
From comments in the source code, it looks like this was introduced by @ttatsuno. Perhaps this used in AGK?