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

Add fortran flag for aarch64 #3673

Merged
merged 1 commit into from Jan 19, 2018

Conversation

mrodozov
Copy link
Contributor

AArch64 builds were failing because of LoopTools failing to compile some fortran code, where the error log was suggesting to use the flag so to avoid the build error.

@mrodozov
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 18, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/25520/console

@mrodozov mrodozov removed the request for review from davidlt January 18, 2018 17:58
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mrodozov (Mircho Rodozov) for branch IB/CMSSW_10_0_X/gcc630.

@cmsbuild, @smuzaffar, @gudrutis, @mrodozov can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

@davidlt
Copy link
Contributor

davidlt commented Jan 18, 2018

Does herwigpp have a testsuite? If yes, could you run it before and after the change, e.g. on x86_64? Just to figure out what could be potential impact.

Also could you share the actual error for the record?

@cmsbuild
Copy link
Contributor

-1

Tested at: db8a8f0

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3673/25520/summary.html

I found follow errors while testing this PR

Failed tests: AddOn

  • AddOn:

I found errors in the following addon tests:

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3673/25520/summary.html

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-3673/1325.7_TTbar_13_94XNanoAODINPUT+TTbar_13_94XNanoAODINPUT+NANOEDMMC2017

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2467599
  • DQMHistoTests: Total failures: 206
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2467224
  • DQMHistoTests: Total skipped: 169
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 1.04999999992 KiB( 22 files compared)
  • Checked 110 log files, 9 edm output root files, 26 DQM output files

@mrodozov
Copy link
Contributor Author

mrodozov commented Jan 19, 2018

The actual error was a fortran lib fails to build in a subpackage of Herwig called LoopTools

PPF77    D/libHwLooptoolsXFC_la-Dget.lo
D/D0func.F:486:34:

  parameter (nz2 = -2147483648) ! O'20000000000'
                                  1
Error: Integer too big for its kind at (1). This check can be disabled with the option -fno-range-check
D/D0func.F:491:27:

  parameter (nz2p1234 = nz2 + p1234)
                           1
Error: Parameter 'nz2' at (1) has not been declared or is a variable, which does not reduce to a constant expression
D/D0func.F:495:27:

and after the error the program 'suggests'

This check can be disabled with the option -fno-range-chec

so that's why we've put the flag. I'm reading the manual of Herwig now to find if there are tests https://herwig.hepforge.org/index.html
The code trying to compile can be seen here:
https://na62-sw.web.cern.ch/sites/na62-sw.web.cern.ch/files/doxygen/d5/dde/LoopTools214_2src_2LoopTools214_2build_2D0funcC_8F_source.html
although seems this is an older version.

@mrodozov
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 19, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/25527/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@mrodozov
Copy link
Contributor Author

+externals

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next IB/CMSSW_10_0_X/gcc630 IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@mrodozov
Copy link
Contributor Author

@Andrej-CMS We integrated Herwig in 10_0_X but we have now this error that the PR fixes, could you ask the developers why do we have it in aarch64 or maybe for any other way to fix it ?
Cheers,
M.

@Andrej-CMS
Copy link
Contributor

@mrodozov

I just wrote an email to the Herwig authors. I will let you know when I get an answer.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3673/25527/summary.html

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-3673/1325.7_TTbar_13_94XNanoAODINPUT+TTbar_13_94XNanoAODINPUT+NANOEDMMC2017

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2467599
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2467429
  • DQMHistoTests: Total skipped: 169
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 1.14000000001 KiB( 22 files compared)
  • Checked 110 log files, 9 edm output root files, 26 DQM output files

@cmsbuild
Copy link
Contributor

lets merge it for now (to get arch64 ibs) as changes only effects arch64. Once we have the proper fix from herwigpp developers then we can include that.

@cmsbuild cmsbuild merged commit 91538c4 into cms-sw:IB/CMSSW_10_0_X/gcc630 Jan 19, 2018
@Andrej-CMS
Copy link
Contributor

Andrej-CMS commented Jan 19, 2018

@ALL

I just put the answer of the Herwig authors here. Is there a way of giving them access to the test account, so they can debug it?

Hi Andrej,

First of all, aarch64 is an architecture we do not currently support
with testing in any way, mainly for lack of test machines. If you have a
way of giving us access to a test account, I can investigate a bit more.

The rest is therefore just an initial guess...

We use a copy of Looptools http://www.feynarts.de/looptools/ internally,
and the linking of that fortran code to the C++ was often tricky to get
right, especially with differing default sizes of numeric types. Your
error is in the pure fortran part, though, so not directly related to
the linking that Herwig does. Can you try compiling Looptools from the
original source above, and see if that works or not. It may be that the
problem is there upstream already.

I looks like on aarch64 the default fortran 'integer' (that your
compiler chooses) is somehow not big enough to hold -2147483648 (usually
this is the minimum that a signed 32-bit integer can be)

Specifying the -fno-range-check will silence the error, but will
potentially write a nonsense value into the variable. This is not a good
way to solve this problem until you know what's going on that causes the
error.

See you,

David

@Andrej-CMS
Copy link
Contributor

Dear @ALL,

Is there any statement on the possibility of giving a test account to the Herwig authors, so they can debug this issue?

Kind regards,
Andrej

@fabiocos
Copy link
Contributor

@Andrej-CMS I have punt this point into the ORP agenda for January 30.

@perrozzi
Copy link

@Andrej-CMS is there any Herwig author able to access lxplus? We discuss in the meeting that this should be a sufficient condition

@Andrej-CMS
Copy link
Contributor

Dear @perrozzi , @ALL,

I talked to the authors . They can access lxplus. It depends on the number of users of the Aarch64 architecture whether they want to put manpower into this issue.

How important is the support for this architecture for CMS? If this is an important issue, I think we should setup an email thread with the authors on this issue.

Kind regards,
Andrej

@smuzaffar
Copy link
Contributor

@smuzaffar
Copy link
Contributor

@Andrej-CMS
Copy link
Contributor

Hello @smuzaffar ,
I made the authors aware of this PR and the new warning.
lets see what they have to say.

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

7 participants