Make improvements

[matt-pharr]

Improvements made to the make workflow. All makefile infrastructure now consolidated in GPEC/install. The new make system is set up to:

• automatically compile any new .f .F .f90 or .F90 files you create
• keep track of dependencies (this feature was implemented before but unused and out of date)
• run parallelized with make -j (number of threads)

On my machine, it compiles the entire project with FFLAGS=-O0 and make -j 8 in under 3 seconds, not including harvest download time. With -O3, it takes ~12 seconds. This is a significant speed up from the single-threaded build system before.

The new build system is broken down into several .inc files to keep it bite-sized with all of the makefiles in one place -- we have DEFAULTS.inc, which remains unchanged. There is RULES.inc which sets the compiler flags for each sub-module. There is SOURCES.inc (which should seldom ever need to be edited), which gets lists of all the fortran/c sources in each sub-module directory. There is TARGETS.inc which contains the actual build targets. Finally, there is dependencies.inc, where there is an entry for every source file that lists the dependencies of that file (i.e. what needs to be completely compiled before it).

Here is an example old makefile, the one for gpec, broken down into where each of these parts live now:

!============================ This line goes in install/makefile
include ../install/DEFAULTS.inc

!============================ These variables are set in install/RULES.inc, along with the .f.o replacements
IFLAGS = -I../equil -I../vacuum -I../harvest -I../pentrc -I../coil -I../slayer -I$(MATHINC) -I$(NETCDFINC)
F90 = $(FC) $(FFLAGS) $(OMPFLAG) $(IFLAGS) $(RECURSFLAG) $(LEGACYFLAG)
export FFLAGS # for sub-makes

.f.o:
	$(F90) -c $*.f

.F.o:
	$(F90) -c $*.F


!============================ These variables, as well as the targets, go in install/TARGETS.inc
LIBDIR = ../lib

LIBS = \
	-lpentrc \
	-llsode \
	-lequil \
	-lvac \
	-lharvest \
	-lslayer \
	-lcoil \

OBJS = \
	gpglobal.o \
	ismath.o \
	idcon.o \
	rdcon.o \
	gpeq.o \
	gpresp.o \
	gpdiag.o \
	gpvacuum.o \
	gpout.o \
	gpec.o

# targets

all: pentrc coil gpec

# takes care of harvest, equil, lsode, etc.
pentrc:
	cd ../pentrc; make all

coil:
	cd ../coil; make


# decide if we need to update the version
# note gpec keeps track of its own version, which can differ from the synced dcon & pentrc one
# this enables faster compiling during development
ifeq ($(wildcard version.inc),)
version.inc: force
	@echo ">>> Creating version file"
	@echo "      CHARACTER(len=*), PARAMETER :: version ='"`git describe --tags`"'" | tee version.inc
else ifeq ($(shell grep -F "'"`git describe --tags`"'" version.inc),)
version.inc: force
	@echo ">>> Updating version file"
	@echo "      CHARACTER(len=*), PARAMETER :: version ='"`git describe --tags`"'" | tee version.inc
else
version.inc:
	@echo ">>> Git version unchanged!"
endif

force:

gpec: $(OBJS)
	$(F90) -o gpec $(OBJS) -L$(LIBDIR) $(LIBS) -L$(MATHDIR) $(MATHLIBS) -L$(NETCDFDIR) $(NETCDFLIBS) $(NETCDF_EXTRA_LIBS) $(LDFLAGS)
	mkdir -p ../lib
	ar -r ../lib/libgpec.a *.o
	mkdir -p ../bin
	cp -f gpec ../bin

!============================ There is now only one make clean, and it is in install/makefile
clean:
	rm -f *.o *.mod *.out *.bin gpec *.original


!============================ These dependencies now go in install/DEPENDENCIES.inc
# dependencies

gpglobal.o: ../equil/fspline_mod.mod ../equil/bicube_mod.mod
ismath.o: gpglobal.o  ../equil/local_mod.mod
idcon.o: gpglobal.o ismath.o
rdcon.o: gpglobal.o ismath.o idcon.o
gpeq.o: idcon.o ismath.o
gpresp.o: gpeq.o
gpvacuum.o : gpeq.o
gpdiag.o: gpresp.o gpvacuum.o
gpout.o: gpdiag.o gpresp.o gpvacuum.o ../coil/field.o ../coil/coil.o \
    ../equil/equil_mod.mod ../equil/equil_out_mod.mod \
    ../pentrc/pentrc_interface.mod ../pentrc/inputs.mod \
    ../slayer/gslayer_mod.mod \
	version.inc 
gpec.o: gpdiag.o gpout.o rdcon.o

[logan-nc]

I've talked through this with @matt-pharr and am happy with the changes. The speedup is a clear benefit, a number of the automations should help clarity. The additional elements we agreed to include are

• Add some documentation of what to check/update when adding/developing in a subdirectory into the development section of the README.md
• Fix a remaining dependency bug we found
• Investigate adding netcdf as a git submodule, since this is the most common complication in DEFAULTS.inc trying to find the right library and include directories.

For the last one, please indicate in a comment here whether this PR should wait for that or not

[logan-nc]

@matt-pharr, I pulled, cleaned and attempted a make. Here are some observations:

• It starts off with a Defaults tailored for platform NULL on host NULL, which looks scary like something it needs is missing. What is the point of this line? Why can't it recognize I am on a macbook? What can it recognize?
• It didn't end with copying everything into ../bin like it used to. The terminal output is so huge it's impossible to track as it goes... so I usually rely on the last bit to show me which ones worked and which didn't. Is there a way to print a nice summary at the end? Note, that if the user did not do make clean then just checking what is in ../bin is not enough, since it may have old ones for the ones that didn't compile.

[logan-nc]

Interestingly, I compiled without -j and did not run into any dependency issues like what you saw when we tried in the meeting

[logan-nc]

I also got a number of,

ld: warning: directory not found for option '-L/usr/X11R6/lib64'

which led me to find that the XDRAW_LIBDIRS seem to be hardcoded. It looks like it is the same hardcoding as the old xdar/makefile, so maybe I just didn't pay attention to these warnings before? Edit: Yes, it shows up once when I compile the develop branch. So this is fine here I suppose.

[matt-pharr]

@logan-nc I have successfully added netcdf and openblas as git submodules. The xdraw libs do indeed need better logic...

I also added an option to clone and install netcdf/openblas to an external directory, which should make setting up gpec on a new cluster/machine quick and easy.

You can test this by doing DEPSINSTALLDIR = /custom/path/to/libraries make. In /custom/path/to/libraries it will make a bin, lib, include, etc so you can export NETCDFHOME=/custom/path/to/libraries and share them across different versions of GPEC.

I am flushing out the documentation by adding summaries at the top of each makefile .inc, and will add some bullets to the documentation webpage. After that, this should be ready for a merge.

[logan-nc]

I am flushing out the documentation by adding summaries at the top of each makefile .inc, and will add some bullets to the documentation webpage. After that, this should be ready for a merge.

@matt-pharr I am still waiting for your "all done" on this one since it still says "draft". It would be good to not let it hang too long

[matt-pharr]

@logan-nc noted. I am experimenting with one other option that would lessen the speed improvements but would improve the output clarity and keep the original makefile structure, let's discuss next Monday once I have had some free time to fiddle with it.

[matt-pharr]

I also believe it should be relatively straightforward to tackle #208 in this PR.

[matt-pharr]

@logan-nc I have decided #208 should be tackled in a separate PR so we can get this merged in, I checked and I should have comments on every document explaining everything. Just need to add to the documentation webpage and this will be ready for merge tonight.

[logan-nc]

@matt-pharr is this ready for review? Is the top comment summary still accurate and complete?

[matt-pharr]

@logan-nc Yes, this should be ready. The top comment summary is still accurate.

[parkjk]

I would also like to review this PR on our side. Maybe it is useful if @matt-pharr debriefs it to @gwanggeunseo or @satelite2517 so that we can take benefits right away in the cluster and macbook.

[matt-pharr]

@parkjk I'd be happy to. Is anyone available to meet this evening US time / tomorrow morning Korea time?

[parkjk]

Thanks. Yes there should be. We will come back to you soon.

[satelite2517]

@matt-pharr Would 7 pm EST work for you? Thanks!

[parkjk]

Matt, please arrange your convenient time of your evening today and invite @satelite2517 and @gwanggeunseo. At least Sunjae will join and follow up. Thanks!

[matt-pharr]

Yes, that works great. To what email should I send a zoom invite? @satelite2517

[satelite2517]

I’ve just sent the Zoom link with your email(matthew.pharr@columbia.edu). Just let me know if it does not works. Thanks