Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add fftpack package #40

Closed
wants to merge 2 commits into from
Closed

add fftpack package #40

wants to merge 2 commits into from

Conversation

zoziha
Copy link
Contributor

@zoziha zoziha commented May 8, 2021

FFTPack aims to provide an easily usable package of functions using FFTPack library (Fortran 77).

This repo contains two libraries:

  1. FFTpack library
    Contains FFT functions, useful functions like: fft, ifft, fftshift, and etc.
  2. Forlab library
    Forlab is a Fortran module that provides a lot of functions for scientific computing mostly inspired by Matlab and Python's package NumPy.
    Useful functions like: disp, ones, eye, and etc.

Thank you, keurfonluu!!😘👍

FFTPack aims to provide an easily usable package of functions using FFTPack library (Fortran 77).

This repo contains two libraries:
1. FFTpack library
   Contains FFT functions, useful functions like: `fft`, `ifft`, `fftshift`, and etc.
2. Forlab library
   Forlab is a Fortran module that provides a lot of functions for scientific computing mostly inspired by Matlab and Python's package NumPy.
   Useful functions like: `disp`, `ones`, `eye`, and etc.
Thank you!!😘👍
@zoziha
Copy link
Contributor Author

zoziha commented May 8, 2021

I want to pull a request with fftpack package, but fftpack package relies on the fpm build --flag '-fallow-argument-mismatch' command (see NCAR/ncl#123) when your gfortran version > 10.
What should I do?

@certik
Copy link
Member

certik commented May 10, 2021

See fortran-lang/fpm#443 for a general handling of -fallow-argument-mismatch.

Since fftpack is quite a foundational library, I would suggest we maintain it under the fortran-lang organization itself.

I would suggest that as the fftpack package we put something along these lines: https://github.com/certik/fftpack, i.e., the original source code. And that we only do changes / fixes if needed.

How are the sources at https://github.com/keurfonluu/FFTPack related to the original sources at netlib (which should be the same as https://github.com/certik/fftpack)?

@zoziha
Copy link
Contributor Author

zoziha commented May 10, 2021

Yes, I also agree with that the fortran-lang organization will be a better and more official platform that can attract a lot of people to participate.

Has Fortran-lang/fpm#443 been implemented? Or still in planning?
I remember that netlib/fftpack(http://www.netlib.org/fftpack/) sources seem to have only one-dimensional fft, and the ncar/fftpack(https://www2.cisl.ucar.edu/resources/legacy/fft5) developed by ncar covers two-dimensional, multi-dimensional Fourier transform, but they are not friendly to beginners, and there is no universal and easy-to-use interface, similar to fft,ifft, fftn, ifffn, fftshift, ifftshift and other well-known functions.
(Also see fortran-lang/stdlib#21 (comment) )

So a package with a universal and easy-to-use interface allows us to use fftpack easily. I understand that there are:

  1. QcmPlab/SciFortran
  2. keurfonluu/FFTPack

QcmPlab/SciFortran has set up a very complete one-dimensional, two-dimensional, multi-dimensional FFT function interface, such as fft, fftn, fftshift and other functions.
(See https://github.com/QcmPlab/SciFortran/tree/master/src/SF_FFT)👍

keurfonluu/fftpack sets up some related FFT function interfaces, but does not set multi-dimensional fft function interfaces, such as fftn (we can make up for it later👌); but it sets up other interesting interfaces, such as filter, conv(convolution), czt(the Chirp Z- transform), hilbert (Hilbert transform) and other effective functions.
(See https://github.com/keurfonluu/FFTPack/blob/master/src/fftpack.f90)✨

I think we can develop a simple and easy-to-use fftpack interface package under the fortran-lang organization, like https://github.com/certik/fftpack.

@zoziha
Copy link
Contributor Author

zoziha commented May 10, 2021

keurfonluu/Forlab(https://github.com/keurfonluu/Forlab/blob/master/src/lib/forlab.f90) is a Fortran module that provides a lot of functions for scientific computing mostly inspired by Matlab and Python's package NumPy.

I really look forward to the easy-to-use packages available for Fortran. I think the keurfonluu/forlab package is very powerful and easy to use. Maybe we can separately introduce the keurfonluu/forlab package into fpm-registry later.

@zoziha
Copy link
Contributor Author

zoziha commented May 10, 2021

Of course, I think you can also pass my PR this time 🤪 to make fftpack immediately available in fpm. Then we will migrate the existing codes to https://github.com/certik/fftpack, which may last for a week or so~. After the migration is successful, we can modify fpm-registy again.
I am new to Fortran, I am currently doing time series analysis and need to use fft, I will try to improve fftpack, I think it’s a fun job :)

@certik
Copy link
Member

certik commented May 10, 2021

Btw, there is also the ffte package:

which is 3D and in parallel and works really well based on my older benchmarks. We should package it also.

Regarding fftpack, here is how you can do a 3D FFT from the 1D FFT:

If you have time, do you want to do a video call to brainstorm how to best move forward? I can help with testing and guidance, I just don't have time to do all the work, but it looks like you might have some time.

@awvwgk
Copy link
Member

awvwgk commented May 10, 2021

Given the discussion in #8 I'm not sure if it is a good idea to add a package to the registry with the intent to replace it later.

@certik
Copy link
Member

certik commented May 10, 2021

No, we should not add a package to the registry and replace it. Rather I want to do a video call with @zoziha and others and figure out what the best way forward is, as a community, and then we'll work on that.

@zoziha
Copy link
Contributor Author

zoziha commented May 10, 2021

I am personally shy, and my English is not very good, it is difficult to make a video call with you, I am sorry.
In addition to video calls, I really can't find an more efficient and fast way to communicate. Maybe I can attend to join the Fortran Monthly Call.
I think my ability is limited (I am still new to Fortran and programming), but I am willing to actively participate in the construction of fftpack. Yes, I have more time in the next few years.

I think we use existing resources (mainly the efforts of the predecessors) to build a more complete fftpack package interface (for easy of use and as a fpm package), there should be no big problem👀.
For other packages such as ffte, we can plan slowly. Maybe we can build a usable fftpack first (I am planning to do this @https://github.com/certik/fftpack), and constantly improve it, at the same time began to consider the introduction of other fft packages.

@certik
Copy link
Member

certik commented May 10, 2021

@zoziha no problem. Let's just discuss here then. Let's have a fortran-lang package for fftpack, and let's worry about ffte separately. For fftpack, I would suggest:


I have rewritten fftpack to more modern Fortran here:

https://github.com/certik/hfsolver/blob/b4c50c1979fb7e468b1852b144ba756f5a51788d/src/fourier.f90#L120

which simplifies some of the routines. I tried to keep the same performance. The question is, should we modernize fftpack in this way?

I am thinking there is value in preserving the original code and just keeping it as is, as much as possible. Just writing some more modern interfaces on top, as I mentioned above. And perhaps have my modern Fortran modernization as another package.

@rouson
Copy link

rouson commented May 10, 2021

@certik Regarding adding "Fortran 90 interface", is there a specific reason to call out "Fortran 90?" I hope to live long enough see the day that it becomes common practice to either simply write "Fortran" or to write "Fortran X", where X is the most recent standard whenever the name is written. It seems that X=90 made such a big splash that it has become a default part of the language name in so many contexts. That has the unfortunate side effect that so many great things that have happened in the last three decades don't get much notice or use.

@zoziha an even more modern approach would separate the interface from the implementation by putting the interface bodies in modules and the procedure definitions in submodules. What I see as the greatest advantage of this approach is that the user need not read the entire implementation just to see how to use the code. Interface information -- possibly supplemented by a few FORD-style comments would suffice for most users.

@certik
Copy link
Member

certik commented May 10, 2021

@rouson good point about Fortran 90. I can just say modern Fortran interface. FFTPACK is written in F77 style with loose subroutines with more of a "lapack" style API, and having it in more modern Fortran multi dimensional arrays with a simpler API would help. That's all.

@zoziha
Copy link
Contributor Author

zoziha commented May 11, 2021

  • Let's collect the original sources from netlib. Commit.

I don't know if it is advisable to download fftpack1.0 code from netlib, it is the original, and NCAR has developed fftpack1.0 to fftpack5.1, which includes a complete multi-dimensional fft function (see https://www2.cisl.ucar.edu/resources/legacy/fft5).
Later John Burkardt converted Fortran77's fftpack5.1 into Fortran90 (single precision and double precision versions).
(see https://people.math.sc.edu/Burkardt/f_src/fftpack5.1/fftpack5.1.html)

I think our choice is to use NCAR's Fortran77 format ncar/fftpack5.1 or John Burkardt's Fortran90 format John/fftpack5.1, instead of netlib/fftpack1.0.
Both QcmPlab/SciFortran and keurfonluu/FFTPack use John Burkardt's Fortran90 format (double precision) John/fftpack5.1.

I also tend to use the latest ffpack content John/fftpack5.1, if something goes wrong, we can fix it.
My question is whether we need to set two precision versions (single precision and double precision) of John/fftpack5.1, or let the compiler choose by compilation flags like nacr/fftpack5.1.

@certik
Copy link
Member

certik commented May 11, 2021

Thanks. I need to see which one to use. One thing that I would like to keep is performance. We should benchmark it. Let me try to review tomorrow and get back to you with some ideas.

@LKedward
Copy link
Member

See also https://github.com/brocolis/fftpack which is already an fpm package. I'm not sure which version of fftpack it is, but it doesn't seem to need -fallow-argument-mismatch.

@certik
Copy link
Member

certik commented May 17, 2021

See also https://github.com/brocolis/fftpack which is already an fpm package. I'm not sure which version of fftpack it is, but it doesn't seem to need -fallow-argument-mismatch.

I wonder if we should start from this version and move it under fortran-lang?

@zoziha
Copy link
Contributor Author

zoziha commented May 17, 2021

See also https://github.com/brocolis/fftpack which is already an fpm package. I'm not sure which version of fftpack it is, but it doesn't seem to need -fallow-argument-mismatch.

I am sure that https://github.com/brocolis/fftpack is fftpack1.0 from netlib/fftpack, with fpm support added. I think we can’ t miss the efforts of ncar/fftpack5.1 and John/fftpack5.1, which have multidimensional FFT and multiprecision, and QcmPlab/SciFortran and keurfonluu/FFTPack have been a great inspiration for interface design.

Because ncar/fftpack5.1 introduced the goto syntax (see https://github.com/keurfonluu/FFTPack/blob/5c66d3bb499e24e2968afa5aac28f80fc88f8f2b/src/fftpack5.f90#L1579) when designing the multidimensional FFT interface, resulting in parameter mismatches, we need -fallow-argument-mismatch (It seems that brocolis/quadpack also has this problem), which we can fix.

If we start with fftpack1.0, we will have to go through the nacr/fftpack5.1 phase at a relatively high cost.😢

@certik
Copy link
Member

certik commented May 17, 2021

@zoziha got it, thanks for the pointers. Here is what I would suggest: let's start with fftpack1.0, then commit over it the fftpack5.1 and then fix the -fallow-argument-mismatch issue. Also I really want to benchmark to ensure the performance is still good. That way we have it in git history and can see what the differences are.

@zoziha
Copy link
Contributor Author

zoziha commented May 17, 2021

I don't know much about licenses. Inspired by #43 (comment), fftpack5.1 may have copyright issues? https://www2.cisl.ucar.edu/resources/legacy/fft5/license

@ivan-pi
Copy link
Member

ivan-pi commented May 17, 2021

I don't know much about licenses. Inspired by #43 (comment), fftpack5.1 may have copyright issues? https://www2.cisl.ucar.edu/resources/legacy/fft5/license

Oddly enough, that license text looks like a combination of BSD-3 licenses terms and the MIT license disclaimer.

The important sentence however is: Redistribution and use of the Software in source and binary forms, with or without modification, is permitted...

So we are free to modify and distribute as desired, as long as the copyright and license text are preserved. 👍

Edit: if I understand correctly the rules of the SPDX license expressions we could use the combination BSD-3-Clause AND MIT to achieve the same license terms. The SPDX license matching guidelines might offer some additional insights. A second (and perhaps easier) option is to consult with NCAR directly.

Edit 2: here is some demonstration of the equivalence (I swapped the first and third conditions in the FFTPACK license)

terms_part

disclaimer_part

@ivan-pi
Copy link
Member

ivan-pi commented May 17, 2021

Also I really want to benchmark to ensure the performance is still good.

Have you got some benchmark codes that could be adapted?

Apart from some micro-benchmarks where N repetitions of a FFT are performed and timed, it would be nice to have some more representative benchmarks of applications from signal processing and physical simulations (pseudo-spectral methods).

Some examples I have encountered before that can be written fairly easily in Fortran are:

  • 1-D Burgers equation
  • 1-D Korteweg–De Vries equation
  • 1-D acoustic wave equation
  • 2-D elastic wave equation
  • 2-D turbulent flow of an incompressible fluid (vorticity-stream function formulation)

@milancurcic
Copy link
Member

Questions:

  1. Considering different flavors / editions of fftpack, should this package be called fftpack5.1 or something else? I worry about the user who does [fpm-search](https://github.com/brocolis/fpm-search) fft gets two or more fftpacks in the results and then has to study each repo to understand the differences and which to use.
  2. Considering that forlab is an fpm package, should it be excluded from the fftpack source, and used as a dependency?

@certik
Copy link
Member

certik commented May 24, 2021

Ideally we should converge on just having one fftpack that works for everybody.

@zoziha
Copy link
Contributor Author

zoziha commented May 25, 2021

Ideally we should converge on just having one fftpack that works for everybody.

I think it’s right to build fftpack in https://github.com/certik/fftpack, but I want to build fftpack4 (netlib/dfftpack1.0) in my own repo(zoziha/modern_fftpack4; zoziha/modern_fftpack4/tree/feature/multiple_precision), which already supports fpm and multi-precision polymorphic interfaces, and then use submodule syntax to hide the internal implementation, and add easy-to-use fft interface. This will be a trial or test of mine, and then I will quickly develop fftpack5.1 in the same way and submit it to https://github.com/certik/fftpack.
I think this will not cause fftpack4 to pollute fftpack5.1. There is a big difference between them and they should be separated.

@milancurcic
Copy link
Member

So, what do we do here now that there is https://github.com/fortran-lang/fftpack? Should they both be added to the registry under different names? Or only the fortran-lang one? I'm not clear what's the difference, I'm an FFT n00b. :)

@zoziha
Copy link
Contributor Author

zoziha commented Aug 19, 2021

We can close this PR, and we only submit the fortran-lang/fftpack package that will be maintained?
keurfonluu/fftpack is based on John/fftpack5.1 (GPL license)
fortran-lang/fftpack is based on netlib/dfftpack1.0(fftpack4.0) (public domain)
For details, see fortran-lang/fftpack#1 (comment)
image

@certik
Copy link
Member

certik commented Aug 19, 2021

Yes, let's just submit the fortran-lang fftpack only for now and maintain it as a community. Let's make sure it works for everybody, people will probably report use cases, and let's fix them. That way there will become one canonical fftpack that people can use and we can all maintain it as a community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants