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

[PETSc] Upgrades petsc to 3.15.2 and enables multiple precision, scalar, and int types #3249

Merged
merged 6 commits into from
Jul 14, 2021

Conversation

jkozdon
Copy link
Contributor

@jkozdon jkozdon commented Jun 30, 2021

  • Upgrades PETSc to 3.15.2
  • Allows for 8 different build options:
    • Scalar Types: complex and real
    • Real Types: Float64 and Float32
    • Int Types: Int32 and Int64

The libraries named:

  • libpetsc_Float64_Real_Int64
  • libpetsc_Float32_Real_Int64
  • libpetsc_Float64_Complex_Int64
  • libpetsc_Float32_Complex_Int64
  • libpetsc_Float64_Real_Int32
  • libpetsc_Float32_Real_Int32
  • libpetsc_Float64_Complex_Int32
  • libpetsc_Float32_Complex_Int32

Additionally, there is a library libpetsc which is the same as libpetsc_double_real_Int32 (the current default version of PETSc_jll).

@jkozdon jkozdon marked this pull request as draft June 30, 2021 23:46
@giordano
Copy link
Member

giordano commented Jul 1, 2021

i686-linux-* and x86_64-linux-* is an incredibly bizarre set of platforms that would fail, since these are the only platforms for which we can actually do native compilation (i686-linux-musl is actually broken for C++ and Fortran, but can run pure C applications)

@jkozdon jkozdon force-pushed the jek/petsc branch 7 times, most recently from 94e5884 to 5e10602 Compare July 1, 2021 22:56
@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 1, 2021

i686-linux-* and x86_64-linux-* is an incredibly bizarre set of platforms that would fail, since these are the only platforms for which we can actually do native compilation (i686-linux-musl is actually broken for C++ and Fortran, but can run pure C applications)

Any tips for working around this?

@giordano
Copy link
Member

giordano commented Jul 1, 2021

If you haven't done that yet, I'd suggest to locally enter the build environment and read the log files, to understand what's going on:

julia build_tarballs.jl --debug --verbose x86_64-linux-gnu-libgfortran5

PETSc has a super custom build system, so I can't easily try and guess something without reading the logs. But the fact that only these platforms fail is very remarkable, it's the exact opposite of what I'd expect in general: platforms that can do native compilation should be easier to deal with in our cross-compilation framework.

@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 2, 2021

If you haven't done that yet, I'd suggest to locally enter the build environment and read the log files

Will do. I misread you previous message, and thought that you were suggesting that failure was likely.

@jkozdon jkozdon changed the title [PETSc] Upgrades petsc to 3.15.1 and enables multiple precision and scalar types [PETSc] Upgrades petsc to 3.15.1 and enables multiple precision, scalar, and int types Jul 2, 2021
@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 2, 2021

Hmm... doesn't even work without my changes so the problem is some change outside of PETSc. Going to try to enlist someone who is better at build systems than me.

In case anyone is interested here is what I am doing to just try i686-linux-gnu-libgfortran3

One of the weird things to me is that there are errors like
/workspace/destdir/lib/libstdc++.so: undefined reference to __divmoddi4@GCC_7.0.0'`

@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 2, 2021

Folks on the petsc mailing list think that there is a problem with the compiler install given the reference to __divmoddi4@GCC_7.0.0' in libstdc++.so.

Could there be some issue with CompilerSupportLibraries_jll? I did notice that it was updated with #3198 but MPICH hasn't been update yet. When I tried my own newly deployed version of MPICH this didn't help though, so I guess that's not an issue 🤷‍♂️ .

If I turn off the c++ interface with --wtih-cxx=0 things seem to build fine, the downside of this is that future building with external packages might be hindered.

@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 2, 2021

I have reverted all my commits and just rolled back the CompilerSupportLibraries_jll to right before #3198 which allows the current version of PETSc_jll to build. (Prior to this the build of PETSc_jll on master fails.)

So it seems that there is something that broken with the c++ libraries with #3198(?), though #3198 seems so minimal I have no idea what could be wrong.

Not sure how to proceed.... Thoughts?

@giordano
Copy link
Member

giordano commented Jul 2, 2021

Uhm, the fact that libstdc++ from GCC 11 may be relevant is interesting and concerning, since that's what will be used in Julia v1.7. Rolling back CompilerSupportLibraries_jll isn't much useful, that's a set of libraries that are really important only at runtime, not much during the build (compilers shard already come with those libraries). I need to investigate what's going on here, but it'll take a few days.

@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 2, 2021

I need to investigate what's going on here, but it'll take a few days.

Thanks.

@giordano
Copy link
Member

giordano commented Jul 9, 2021

For the record, I believe I tracked down the issue to the simple reproducer I reported in JuliaPackaging/BinaryBuilderBase.jl#163 and I already have a fix for that, but I need to write the tests and that may take a bit as it's surprisingly complicated to do in a nice way (by "nice" I mean in a way that doesn't force me to download the CompilerSupportLibraries_jll tarball every time). I'm moderately confident that should fix also the issues we have here (at least the symptoms are the same) but I haven't tried to build PETSc with that. Thanks for the patience!

@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 9, 2021

@giordano Thanks for the update! No rush, I appreciate you taking a look.

@giordano giordano closed this Jul 12, 2021
@giordano giordano reopened this Jul 12, 2021
@giordano
Copy link
Member

Ok, it looks like JuliaPackaging/BinaryBuilderBase.jl#163 did solve the issues with i686-linux-*-libgfortran3 🙂

@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 12, 2021

Thanks!

@giordano
Copy link
Member

giordano commented Jul 13, 2021

i686-linux-musl-libgfortran3 and i686-linux-musl-libgfortran4 failed with

[00:17:55] i686-linux-musl_double_real_Int32/obj/sys/logging/plog.o: In function `PetscLogInitialize':
[00:17:55] plog.c:(.text+0xbe8): undefined reference to `__stack_chk_fail_local'
[00:17:55] i686-linux-musl_double_real_Int32/obj/sys/logging/plog.o: In function `PetscLogDump':
[00:17:55] plog.c:(.text+0x2683): undefined reference to `__stack_chk_fail_local'
[00:17:55] i686-linux-musl_double_real_Int32/obj/sys/logging/plog.o: In function `PetscLogView_Detailed':
[00:17:55] plog.c:(.text+0x35c0): undefined reference to `__stack_chk_fail_local'
[00:17:55] i686-linux-musl_double_real_Int32/obj/sys/logging/plog.o: In function `PetscLogView_CSV':
[00:17:55] plog.c:(.text+0x3e8d): undefined reference to `__stack_chk_fail_local'
[00:17:55] i686-linux-musl_double_real_Int32/obj/sys/logging/plog.o: In function `PetscLogView_Default':
[00:17:55] plog.c:(.text+0x7633): undefined reference to `__stack_chk_fail_local'
[00:17:55] i686-linux-musl_double_real_Int32/obj/sys/logging/plog.o:plog.c:(.text.unlikely+0x9a): more undefined references to `__stack_chk_fail_local' follow
[00:17:55] collect2: error: ld returned 1 exit status
[00:17:55] make[3]: *** [gmakefile:113: i686-linux-musl_double_real_Int32/lib/libpetsc.so.3.15.2] Error 1
[00:17:55] make[2]: *** [/workspace/srcdir/petsc-3.15.2/lib/petsc/conf/rules:50: libs] Error 2

The same error with undefined reference to __stack_chk_fail_local was reported in #845 (comment). The workaround there was removing superlu (not really a fix...).

@jkozdon jkozdon changed the title [PETSc] Upgrades petsc to 3.15.1 and enables multiple precision, scalar, and int types [PETSc] Upgrades petsc to 3.15.2 and enables multiple precision, scalar, and int types Jul 13, 2021
@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 13, 2021

The same error with undefined reference to __stack_chk_fail_local was reported in #845 (comment). The workaround there was removing superlu (not really a fix...).

Unfortunately superlu is already turned off. Going to work on trying to debug locally.

@giordano
Copy link
Member

If this is ready, you should mark it so 🙂

However auditor reports a warning about the fact there are two files with the same path, module case: they generate two identical pkgconfig files with names PETSc.pc and petsc.pc: https://gitlab.com/petsc/petsc/-/blob/4204ec6bd09c0f6df882d207f007f8d92faa2086/config/PETSc/Configure.py#L981-L982. This can be a problem on case-insensitive file-systems, especially since Pkg will check the git-tree-sha1 of the unpacked tarball, and this check may fail.

@jedbrown I see you introduced the petsc.pc file: do you think it's a good idea for us to remove the PETSc.pc file?

@jedbrown
Copy link
Contributor

The PETSc installer writes PETSc.pc first and then petsc.pc, which should replace the first if the filesystem is case insensitive. This was intended for backward compatibility, which I don't think Julia cares about.

@giordano
Copy link
Member

Yeah, the fact is that here we create the tarballs on a case-sensitive file-system but will install the tarballs on case-insensitive ones 🙂 But yeah, should we have troubles with other packages not finding PETSc we can fix that

@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 13, 2021

The whole point of the PR was to enable multiple scalar, real, and int type versions of PETSc.

I'm building these right now to make sure that they work fine before releasing the PR

@jedbrown
Copy link
Contributor

Cool, @giordano If you think PETSc upstream should do something differently, let us know. I figure we can remove PETSc.pc at some point in the future, but it would be somewhat disruptive to existing users to do it now.

@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 13, 2021

OK. I think that I am happy with this now.

  • I've re-enabled the mult-lib build
  • Fixed the PR description
  • Cleaned up the commit history some
  • Tested with a local binary build

@jkozdon jkozdon marked this pull request as ready for review July 13, 2021 23:37
@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 14, 2021

Would it be worth also adding debug versions of the library? That blows up the build even more though...

cc: @psanan, @boriskaus, @simonbyrne ?

@boriskaus
Copy link
Contributor

boriskaus commented Jul 14, 2021 via email

@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 14, 2021

Let me see if I can enable these...

@jkozdon jkozdon force-pushed the jek/petsc branch 6 times, most recently from 8d15f81 to 62165b2 Compare July 14, 2021 18:25
@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 14, 2021

It's looking like mumps, superlu, and superlu_dist will take a bit of work to sort out.

I know that superlu didn't work before either in the original PETSc PR #845

@giordano
Copy link
Member

The only thing I'd like to ask is to delete ${prefix}/lib/PETSc.pc at the end of the build, to avoid problems on Windows and macOS, as discussed above. After that, this would be good to go from me.

@giordano giordano enabled auto-merge (squash) July 14, 2021 20:38
@giordano giordano merged commit a2161a5 into JuliaPackaging:master Jul 14, 2021
@jkozdon
Copy link
Contributor Author

jkozdon commented Jul 14, 2021

🎉 Thanks!

@psanan
Copy link

psanan commented Jul 15, 2021

@jkozdon re whether debug builds should be included, this depends on whether users will need the extra checks that debug mode provides. In normal PETSc usage, this adds more checks that Vecs are the right size, pointers are non-NULL, etc.

@giordano giordano mentioned this pull request Aug 16, 2021
simeonschaub pushed a commit to simeonschaub/Yggdrasil that referenced this pull request Feb 23, 2022
…ar, and int types (JuliaPackaging#3249)

* [PETSc] Update PETSc version

* [PETSc] Add `-fno-stack-protector` flag

* [PETSc] Fix MPI library path

* [PETSc] Require GCC 9 for libgfortran5

The linker that comes with GCC 8 emits a warning on `{i686,x86_64}-linux-musl` and that makes configuration script fail.

* [PETSc] Add build with multiple types

 - Real and Complex
 - Int32 and Int64
 - Float64 and Float32

* [PETSc] Delete duplicated PETSc.pc file

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
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.

5 participants