FluidityProject / fluidity

Fluidity
http://fluidity-project.org
Other
365 stars 115 forks source link

Add new Dockerfile for focal #297

Closed tmbgreaves closed 3 years ago

tmbgreaves commented 3 years ago

Somewhat overdue (sorry!) I've pushed a set of packages for focal onto launchpad, and this PR is intended to discuss updates to support focal, hopefully ending up with a focal-supporting merge.

On the package side:

On the docker side:

@stephankramer - please could you cast your eye over the PETSc side of things at some point and check I haven't done anything silly there? Also if you have any thoughts on whether there's any option to use system libtrilinos-zoltan without too much headache from the enormous package, I'd very much appreciate any ideas.

tmbgreaves commented 3 years ago

Actions on me:

gnikit commented 3 years ago

It is worth noting that Fluidity's dependency on HDF5 is strictly limited to version 1.10.x and cannot use v1.12, from my understanding that is because H5hut has not been updated to use HDF5 1.12 yet. As a result, configuration succeeds but compilation fails. It might be useful to add a temporary check in the configure script for the version of HDF5.

If that is the case, one might also need to be aware of this post from the HDF group https://portal.hdfgroup.org/display/HDF5/Software+Changes+from+Release+to+Release+for+HDF5-1.10 discussing the differences and compatibility issues between the 1.10.x versions

stephankramer commented 3 years ago

Wow, thanks @tmbgreaves - that all sounds great, in particular the work on the packages. It's nice to have cleaner petsc install as our basis environment, as it makes it easier to swap out with different versions of it. The changes in the Dockerfile look perfectly sane to me. Do we need to update instructions in the manual? I'll ask @angus-g to have a look as well when he's back - and maybe indeed he could add a check for the specific h5hut dependency as @gnikit pointed out (thanks!) To be honest I'm not too fussed about specifically using libtrilinos-zoltan - I'm guessing it needs tweaking in a similar way as petsc to get the right fortran interfaces? So if it's painful to do it in that package, I'm perfectly happy to stick with what we're currently using. We're sort of relying on a frozen version of zoltan there anyway, as I think the development happens on zoltan2 - so fingers crossed our zoltan keeps on building on newer platforms :crossed_fingers:

tmbgreaves commented 3 years ago

Yes, the libtrilinos package in theory is easy to hack as you just switch on fortran support for the legacy zoltan package, but in practise I found it was a pain to build as it both takes a long time to build the whole thing and fails the testing step on my builds.

tmbgreaves commented 3 years ago

It is worth noting that Fluidity's dependency on HDF5 is strictly limited to version 1.10.x and cannot use v1.12

This is going to cause some fun imminently with the build on Archer2 which only has 1.12. Is there any sensible workaround here that people are already using in such situations, before I start digging?

gnikit commented 3 years ago

It is worth noting that Fluidity's dependency on HDF5 is strictly limited to version 1.10.x and cannot use v1.12

This is going to cause some fun imminently with the build on Archer2 which only has 1.12. Is there any sensible workaround here that people are already using in such situations, before I start digging?

So what I did locally was build HDF5 v1.10.x from source so I guess that is one option.

stephankramer commented 3 years ago

@angus-g : since you know more about h5hut. Any insights?

angus-g commented 3 years ago

The actual required change to H5hut is miniscule, just needing to go to a versioned macro that was superseded. I'll see if we can get this merged in upstream. If necessary though, I think it would be easier to incorporate this patch in Fluidity (if not the h5hut subtree, then part of the build process?) than to have a hard dependency on HDF5 1.10.x...

diff --git a/src/h5core/private/h5_hdf5.c b/src/h5core/private/h5_hdf5.c
index 59b766d..846f892 100644
--- a/src/h5core/private/h5_hdf5.c
+++ b/src/h5core/private/h5_hdf5.c
@@ -110,10 +110,10 @@ iter_op_get_obj_type (
                                name);
                        return H5O_TYPE_UNKNOWN;
                }
-               herr = H5Oget_info(obj_id, &objinfo);
+               herr = H5Oget_info1(obj_id, &objinfo);
        }
        else { // H5L_TYPE_HARD
-               herr = H5Oget_info_by_name(g_id, name, &objinfo, H5P_DEFAULT);
+               herr = H5Oget_info_by_name1(g_id, name, &objinfo, H5P_DEFAULT);
        }

        if (herr < 0) {
diff --git a/src/h5core/private/h5_hdf5.h b/src/h5core/private/h5_hdf5.h
index 2e2d6c3..8ac05b8 100644
--- a/src/h5core/private/h5_hdf5.h
+++ b/src/h5core/private/h5_hdf5.h
@@ -1293,7 +1293,7 @@ hdf5_close_file (
                        hid_t object_id = obj_id_list [i];
                        h5_debug ("Open object: %lld", (long long)object_id);
                        H5O_info_t object_info;
-                       if (H5Oget_info (object_id, &object_info) < 0)
+                       if (H5Oget_info1 (object_id, &object_info) < 0)
                                continue;
                        switch (object_info.type) {
                        case H5O_TYPE_GROUP:
diff --git a/test/testframe.c b/test/testframe.c
index cbd8873..bcafbcc 100644
--- a/test/testframe.c
+++ b/test/testframe.c
@@ -658,7 +658,7 @@ test_open_objects(h5_file_t file, int max_objects)
                H5O_info_t info;
                int i;
                for (i=0; i<nopen; i++) {
-                       H5Oget_info(list[i], &info);
+                       H5Oget_info1(list[i], &info);
                        switch (info.type) {
                        case H5O_TYPE_GROUP:
                                TestErrPrintf("obj%d has type GROUP\n", i);
tmbgreaves commented 3 years ago

The actual required change to H5hut is miniscule, just needing to go to a versioned macro that was superseded. I'll see if we can get this merged in upstream. If necessary though, I think it would be easier to incorporate this patch in Fluidity (if not the h5hut subtree, then part of the build process?) than to have a hard dependency on HDF5 1.10.x...

Thanks Angus - that's hugely useful. I agree it would be good to get this into Fluidity. If it's a safe change for 1.10 and doesn't break backwards compatibility might you be able to PR the change into our h5hut subtree? If not then patches would work just fine.

gnikit commented 3 years ago

Will the versions of GMSH and NetCDF also be bumped up with the upgrade to Focal?

Patol75 commented 3 years ago

For GMSH, #235 has to be merged first I believe. Is that feasible @angus-g @stephankramer?

gnikit commented 3 years ago

@angus-g having used the patch for HDF5-1.12.0 we seem to be getting warnings (see below) which when compiling in parallel caused the linker to fail by not being able to link H5Oget_info1 and H5Oget_info_by_name1. In serial it seems to have compiled/linked successfully.

private/h5_hdf5.h:1296:33: warning: passing argument 2 of ‘H5Oget_info1’ from incompatible pointer type [-Wincompatible-pointer-types]
 1296 |    if (H5Oget_info1 (object_id, &object_info) < 0)
      |                                 ^~~~~~~~~~~~
      |                                 |
      |                                 H5O_info2_t * {aka struct H5O_info2_t *}

Does that also happen to you or is it just me?

gnikit commented 3 years ago

Also there are a few more compiler warnings that I found worrisome and might or might not lead to strange behaviour. To replicate configure with debug and compile.

  1. Deprecated features
makM.f:493:20:

  493 |  200     nF = nF - 1
      |                    1
Warning: Fortran 2018 deleted feature: DO termination statement which is not END DO or CONTINUE with label 200 at (1)
makM.f:546:20:

  546 |  300     nE = nE - 1
      |                    1
Warning: Fortran 2018 deleted feature: DO termination statement which is not END DO or CONTINUE with label 300 at (1)
  1. Deprecated declarations, search for -Wdeprecated-declarations
  2. C binding type warnings search for -Wc-binding-type (probably not important)
  3. Type conversions from doubles to ints, from 8-byte to 4-byte, etc. search for -Wconversion (probably not important)
  4. String truncation, search for -Wcharacter-truncation (possible not important)

It is possible that there are other warnings that should be addressed but I have discarded as trivial

tmbgreaves commented 3 years ago

This has been almost entirely superseded by #308 which contains newer Dockerfiles for both focal and groovy.