Skip to content

Fixes and tweaks for configure.ac#3179

Open
oskooi wants to merge 5 commits intoNanoComp:masterfrom
oskooi:configure_fixes
Open

Fixes and tweaks for configure.ac#3179
oskooi wants to merge 5 commits intoNanoComp:masterfrom
oskooi:configure_fixes

Conversation

@oskooi
Copy link
Copy Markdown
Collaborator

@oskooi oskooi commented Mar 25, 2026

1: Upgrade minimum C++ standard from C++11 to C++17

Line 103: AX_CXX_COMPILE_STDCXX([11], [], [optiona])

C++11 is very old. The [optional] flag means compilation proceeds even without C++11 support. Given Meep is actively developed with modern C++ features, this should be at least C++14 or C++17 and [mandatory]. Note: the [mandatory] option indicates that C++17 is a minimum baseline rather than a strict version lock.

2: Duplicate AC_CHECK_LIB(m, ...) calls

Lines 150 and 233: AC_CHECK_LIB(m, sin) and AC_CHECK_LIB([m],[cos])libm is checked twice with different functions. The second check at line 233 is redundant since -lm is already in $LIBS from line 150. Remove one of them.

3: Inconsistent --with-openmp vs. --enable-openmp semantics

Line 139: Uses --with-openmp (via AC_ARG_WITH) but stores the result in enable_openmp=$enableval. The Autoconf convention is --enable-* for features and --with-* for external packages. OpenMP is a feature, so this should be AC_ARG_ENABLE(openmp, ...). The variable naming is also confusing — the argument is with_openmp but the assignment goes to enable_openmp, then line 140 checks $with_openmp:

Line 139: stores $enableval in enable_openmp
Line 140: but then checks $with_openmp

This works by accident (since $with_openmp is set by AC_ARG_WITH), but enable_openmp is never used.

4: MPICH SEEK_SET workaround is obsolete

Lines 77-99: The MPICH_IGNORE_CXX_SEEK workaround was for MPICH2 and very old MPI implementations. MPICH 3.x (released 2012) fixed this. Remove block and clean up src/fix_boundary_sources.cpp and src/mympi.cpp.

5: Obsolete Guile API checks

Lines 339-362: The checks for SCM_SMOB_PREDICATE, SCM_SMOB_DATA, SCM_NEWSMOB, and scm_make_smob_type test for APIs that have been standard since Guilde 1.8. These can be removed since Guile 2.0+ is a requirement.

6: Document BLAS/LAPACK variables

Added comment explaining $with_blas/$with_lapack are set by the AX_BLAS/AX_LAPACK m4 macros.

7: Add AC_CONFIG_AUX_DIR

Added AC_CONFIG_AUX_DIR([build-aux]), created build-aux/ directory; auxiliary files (install-sh, config.guess, etc.) live there.

8: Clarify FFTW detection

Added comment explaining FFTW2 fallback is intentional (code paths exist in monitor.cpp and casimir.cpp).

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 25, 2026

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.90%. Comparing base (f29a8c7) to head (632fbb9).
⚠️ Report is 117 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3179      +/-   ##
==========================================
+ Coverage   73.81%   73.90%   +0.09%     
==========================================
  Files          18       18              
  Lines        5423     5454      +31     
==========================================
+ Hits         4003     4031      +28     
- Misses       1420     1423       +3     

see 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

configure.ac Outdated

# make sure we enable C++11 support if possible
AX_CXX_COMPILE_STDCXX([11], [], [optional])
# Require C++17 support
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to require this until we actually use C++17 features, which as far as I know we don't? I think the most recent feature we use is std::move, which is C++11. So we should just make C++11 mandatory.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

configure.ac Outdated

AC_ARG_WITH(openmp, [AS_HELP_STRING([--with-openmp],[use OpenMP directives for parallelism])], enable_openmp=$enableval, with_openmp=no)
if test x"$with_openmp" = "xyes"; then
AC_ARG_ENABLE(openmp, [AS_HELP_STRING([--enable-openmp],[use OpenMP directives for parallelism])],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm reluctant to rename this from --with-openmp to --enable-openmp, which could break a lot of people's build scripts for no good reason. In practice, configure scripts are not very consistent on with vs enable

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted to --with-openmp.

AC_CHECK_LIB(m, sin)

# FFTW3 is preferred; FFTW2 (dfftw/fftw) is also supported by Meep
# (see src/monitor.cpp and src/casimir.cpp for FFTW2 code paths).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can probably just delete FFTW2 support at this point.

fi
AC_CHECK_HEADERS([libguile.h guile/gh.h])

# Check how smob types work in this Guile version:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little reluctant to require guile 2; there are still a lot of guile 1.8 installations around.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted Guile 1.8 API checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants