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

Improvements to affinity.c #219

Merged
merged 2 commits into from
May 24, 2023
Merged

Improvements to affinity.c #219

merged 2 commits into from
May 24, 2023

Conversation

ngs333
Copy link
Contributor

@ngs333 ngs333 commented Apr 11, 2023

This PR improves affinity.c by:

  1. replacing affinity.c with one copied from FMS/affinity;
  2. modifies configure.ac to check for sched_getaffinity;
  3. modifies the new affinity.c to #include configure.h.

This has been tested with gcc on analysis (an008) with and without the --with-mpi compilation flag using the canonical compilation method. It has also bee tested on an ubuntu laptop.

I think it would be prudent to build and test on Apple OSX.

FYI, The new affinity.c #includes the generated configure.h file. Somehow FMS is able to compile affinity.c without this change - to my surprise.

check for sched_getaffinity; modified affinity.c to include configure.h.
@ngs333 ngs333 linked an issue Apr 11, 2023 that may be closed by this pull request
@ngs333 ngs333 requested a review from rem1776 April 11, 2023 19:29
@ngs333 ngs333 marked this pull request as ready for review April 11, 2023 19:35
tools/libfrencutils/affinity.c Outdated Show resolved Hide resolved
tools/libfrencutils/affinity.c Show resolved Hide resolved
@ngs333
Copy link
Contributor Author

ngs333 commented Apr 21, 2023

@ceblanton @underwoo
In this PR I was merely following Seths suggestion (in #17 (comment) ). Now the differences between the NCTools affinity.c of this PR and the FMS affinity.c is that a) #include "config.h" was also added to the NCTools one as its needed to compile; and b) some comments that were added for the FMS doxygen system were removed.

@ngs333
Copy link
Contributor Author

ngs333 commented May 3, 2023

@ilaflott , @ceblanton
Ian and Chris, this is the PR which I mentioned could use compiling and running on a Mac. Thanks.

@ilaflott
Copy link
Member

Build on Macbook Air, 202 13inch, macOS 13.3.1 (ventura), intel chip failed. Though to be fair, it seems to fail buildingNOAA-GFDL's master branch with an identical complaint. Of course, I attempted to rebuild/configure etc. following the suggestion in the error message, but to no avail.

checking netCDF Fortran 90 binding available... no
configure: WARNING: Unable to locate a working Fortran netCDF library.  Please specify --with-netcdf-fortran=<LOCATION> where <LOCATION> is the full path to where netCDF is installed.
configure: error: Unable to find NetCDF Fortran library.
make: *** No targets specified and no makefile found.  Stop.

Of course, I made sure i had this library:

[build $] nf-config

This  4.6.0-development has been built with the following features:

  --cc        -> clang
  --cflags    -> -I/usr/local/Cellar/netcdf-fortran/4.6.0_1/include  -g -Wall -Wno-unused-variable -Wno-unused-parameter -O2

  --fc        -> /usr/local/bin/gfortran
  --fflags    -> -I/usr/local/Cellar/netcdf-fortran/4.6.0_1/include -I/usr/local/Cellar/netcdf-fortran/4.6.0_1/include
  --flibs     -> -L/usr/local/Cellar/netcdf-fortran/4.6.0_1/lib -lnetcdff -lnetcdf -lnetcdf
  --has-f90   -> TRUE
  --has-f03   -> yes

  --has-nc2   -> yes
  --has-nc4   -> yes

  --prefix    -> /usr/local/Cellar/netcdf-fortran/4.6.0_1
  --includedir-> /usr/local/Cellar/netcdf-fortran/4.6.0_1/include
  --version   ->  4.6.0-development

Though, for what it's worth, I don't think this means the branch is necessarily at fault. Maybe my env is bunked up because i basically lean on homebrew to manage most of it. But maybe this is enough to say it doesn't pass the "scientist can easily use it with a mac" test.

@ceblanton ceblanton self-requested a review May 24, 2023 21:08
@ceblanton ceblanton merged commit 52fd0d8 into NOAA-GFDL:master May 24, 2023
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.

Improve affinity.c conditional compilation and remove duplicate.
3 participants