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

openmpi: make build reproducible and fix compiler wrappers #58964

Merged
merged 2 commits into from Sep 15, 2019

Conversation

markuskowa
Copy link
Member

Motivation for this change

The build system integrates various time stamps (+user, hostname) into the output thus creating an irreducible build.

The MPI compiler wrappers should point to the compilers used at build time per default. For MPICH this feature is already in the derivation (#44555)

Things done
  • Patch in fixed timestamps, usernames, and hostname
  • Set absolute path names for MPI compiler wrappers.
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

# default compilers should be indentical to the
# compilers at build time

sed -i 's:compiler=.*:compiler=${stdenv.cc}/bin/cc:' $out/share/openmpi/mpicc-wrapper-data.txt
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct in the case of cross-compilation?

Copy link
Member Author

@markuskowa markuskowa Apr 7, 2019

Choose a reason for hiding this comment

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

Cross compilation is still a mystery to me but this probably only works if hostPlatform == buildPlatform. How would I get the compilers of the target platform? cc @Mic92

Copy link
Member

@Mic92 Mic92 Apr 12, 2019

Choose a reason for hiding this comment

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

@markuskowa I think this would be ${targetPackages.stdenv.cc}

Copy link
Member

Choose a reason for hiding this comment

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

But actually there was this one recently introduced: #57611
So instead this would be:

"${pkgsTargetTarget.stdenv.cc}/bin/${pkgsTargetTarget.stdenv.cc.targetPrefix}cc"

Copy link
Member

Choose a reason for hiding this comment

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

@Ericson2314 could maybe also provide the right expression for gfortran.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the insight! Just extrapolating from the expression for gcc:

${pkgsTargetTarget.gfortran}/bin/${pkgsTargetTarget.gfortran.targetPrefix}gfortran

Does that make sense? EDIT: it yields the right result when I build locally.

fix:
* build and configure time stamps
* build and configure user
The MPI compiler wrappers should point to the
compilers used at build time per default.
@bjornfor bjornfor merged commit 3857d3d into NixOS:master Sep 15, 2019
@bjornfor
Copy link
Contributor

Thanks!

@markuskowa markuskowa deleted the fix-ompi-repro branch September 15, 2019 18:33
@abbradar
Copy link
Member

abbradar commented Oct 3, 2019

This semi-breaks cntk because it needs GCC7 on one hand and uses mpic++ on another so replacing stdenv doesn't help. I'd expect mpic++ to use whatever compiler is currently in PATH by default, what is the right way to fix that?

@Mic92
Copy link
Member

Mic92 commented Oct 3, 2019

@abbradar is it not common to set OMPI_CC to set a different compiler?

@markuskowa
Copy link
Member Author

@abbradar setting OMPI_CXX should override the C++ compiler. To avoid problems with different libstdc++ versions you could also override the stdenv of openmpi. That way your application and openmpi are built with the same compiler.

@abbradar
Copy link
Member

abbradar commented Oct 4, 2019

@Mic92 @markuskowa Yep, that's the answer I needed. Thanks!

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.

None yet

6 participants