move include/common to include/finufft_common

[iboB]

include/common is a very... er... common name for an include directory. Since it contains generic files like "common.h", "utils.h", etc it makes it extremely susceptible to include path clashes, where in certain scenarios it would be impossible to #include <common/defines.h> and get the right thing.

I'd say this is bordering on adding code in to a file called stdio.h. I think a library has no place of doing this to the include paths.

This PR moves common to finufft_common to avoid clashes in projects which use the same directory for their common includes.

Perhaps a better suggestion would be to move all headers to a distinct directory (say finufft or fi or flatiron) and get to a structure like like

finufft/
   finufft.h
   common/
   cuda/
   finufft/

... but that's a decision better thought through within the team.

PS I am again trying to sneak the ninja commit of .gitignore from #725

[lu1and10]

... but that's a decision better thought through within the team.

Thanks @iboB, we will discuss in the next team meeting and keep you posted.

[DiamonDinoia]

Hi @iboB,

Do you know what is the best practice for solving this issue? The usual structure used by common big libraries that we should follow? Also, any suggestion on naming here is welcome, I am not great at it.

Thanks,
Marco

[iboB]

The modern approach is to have a "namespace" directory and no headers outside of it. Besides that I don't think there's anything close to a standard or a de facto standard for C or C++.

The desire to keep finufft and cufinufft separate, and also have common code between them, would in practice mean that there are three libraries, so I would suggest the following structure:

finufft/ # could also be flatiron hinting for a bigger flatiron library collection
  common/     # common library
  finufft.h   # common include for cpu 
  finufft/    # cpu version
  cufinufft.h # cuda version
  cufinufft/

Then one would include <finufft/finufft.h> or in case they want something more granular <finufft/finufft/spreadinterp.h>

If you plan on merging the two in a unified library, I'd simplify things by going:

finufft/
  defines.h # common headers directly in here
  utils.h 
  cpu.h     # what is currently finufft.h
  cuda.h    # what is currently cufinufft.h
  cpu/   # further subdirs don't matter that much here
  cuda/  # splicing all headers directly here is also fine
  finufft.h # perhaps a header with an interface
            # where cpu/cuda is a configurable argument

Other things that I personally find appealing:

• Use .hpp for C++ header files and .h for C. Thus if you support a C interface again, you could have sibling files for the two languages <finufft/finufft.h> and <finufft/finufft.hpp>. I am a big fan of this pattern
• Another modern approach that I like is not having headers and sources in separate directories, but instead adding everything in code/finufft (or maybe cpp/finufft or core/finufft since multiple languages are supported here) where each header is a sibling to its corresponding .c/.cpp (if any). Modern CMake file sets and export sets make this really easy to manage and install, and file browsing is much more pleasant.

[DiamonDinoia]

The modern approach is to have a "namespace" directory and no headers outside of it. Besides that I don't think there's anything close to a standard or a de facto standard for C or C++.

The desire to keep finufft and cufinufft separate, and also have common code between them, would in practice mean that there are three libraries, so I would suggest the following structure:

finufft/ # could also be flatiron hinting for a bigger flatiron library collection
  common/     # common library
  finufft.h   # common include for cpu 
  finufft/    # cpu version
  cufinufft.h # cuda version
  cufinufft/

I second this. It is what I use and see in other open source projects.

Then one would include <finufft/finufft.h> or in case they want something more granular <finufft/finufft/spreadinterp.h>

If you plan on merging the two in a unified library, I'd simplify things by going:

finufft/
  defines.h # common headers directly in here
  utils.h 
  cpu.h     # what is currently finufft.h
  cuda.h    # what is currently cufinufft.h
  cpu/   # further subdirs don't matter that much here
  cuda/  # splicing all headers directly here is also fine
  finufft.h # perhaps a header with an interface
            # where cpu/cuda is a configurable argument

I I ma not mistaken the plan is to merge cufinufft and finufft. The reason this has not happened is time I think. My plan is to gradually move functionalities in the common third library and reuse it in both projects until there is no overlap. Then we can do the merge.

Other things that I personally find appealing:

• Use .hpp for C++ header files and .h for C. Thus if you support a C interface again, you could have sibling files for the two languages <finufft/finufft.h> and <finufft/finufft.hpp>. I am a big fan of this pattern

I like this too. I was planning on proposing to the team in the future. Also, some of the linters breaks if the see c++ code in a .h because they think is a c code.

• Another modern approach that I like is not having headers and sources in separate directories, but instead adding everything in code/finufft (or maybe cpp/finufft or core/finufft since multiple languages are supported here) where each header is a sibling to its corresponding .c/.cpp (if any). Modern CMake file sets and export sets make this really easy to manage and install, and file browsing is much more pleasant.

I am not a fan of this because having .cpp and .hpp makes the linting slower for users downstream. Usually includes are specified by folder (-I folder). if the number of files in this directories blows up it makes some linters sluggish or use more ram than needed.

It might not be a problem anymore but experienced issues when multiple of my dependencies go around that pattern.

[DiamonDinoia]

This can be also merged if rebased. I am fine with the renaming for now.