JeffersonLab / qphix

QCD for Intel Xeon Phi and Xeon processors
http://jeffersonlab.github.io/qphix/
Other
13 stars 11 forks source link

Unused variables and signed-unsigned comparisons #110

Open martin-ueding opened 6 years ago

martin-ueding commented 6 years ago

I just turned on some warnings in order to get some better information to track down the build failure on Travis CI regarding the new twisted mass code. The build output is now littered with two types of warnings:

  1. There are lots of undefined variables. It seems that in the index calculations we don't actually need all that stuff. In order to make the code easier to refactor when we implement multiple communication strategies, I would like to remove those variables.

  2. Comparison between signed and unsigned integers. Unfortunately (Stroustup or Sutter have said that) the standard library uses a lot of unsigned integers. Therefore we can either also used unsigned integers in those cases, explicitly cast, or just ignore those warnings. I would assume that just using signed numbers will not do us any harm. The overflow is undefined (contrary to unsigned integers, where it is well defined), but we are not relying on that. So I would assume that one could change our variables to signed.

I think that the first one could be worthwhile, the second might not be that interesting to fix.

What do you think?

bjoo commented 6 years ago

Hi Martin, Agreed, the signed vs unsigned we should just ignore for now.

Thanks for understaking the elimination of the unused vars.

Best, B

⁣Dr Balint Joo, Scientific Computing Group, Jefferson Lab,  Tel: +1 757 269 5339 (Office) Sent from my mobile phone​

On Nov 2, 2017, 11:12 AM, at 11:12 AM, Martin Ueding notifications@github.com wrote:

I just turned on some warnings in order to get some better information to track down the build failure on Travis CI regarding the new twisted mass code. The build output is now littered with two types of warnings:

  1. There are lots of undefined variables. It seems that in the index calculations we don't actually need all that stuff. In order to make the code easier to refactor when we implement multiple communication strategies, I would like to remove those variables.

  2. Comparison between signed and unsigned integers. Unfortunately (Stroustup or Sutter have said that) the standard library uses a lot of unsigned integers. Therefore we can either also used unsigned integers in those cases, explicitly cast, or just ignore those warnings. I would assume that just using signed numbers will not do us any harm. The overflow is undefined (contrary to unsigned integers, where it is well defined), but we are not relying on that. So I would assume that one could change our variables to signed.

I think that the first one could be worthwhile, the second might not be that interesting to fix.

What do you think?

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_qphix_issues_110&d=DwICaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=SC-qvz5njMoFH6cliT5XZQ&m=RaAW5INKXRgn2vQx2DanHx-6eONfiOkeWBp25E-ROKc&s=6J4S98XmrE6efNmChOZj4fBCYf5449Q45N8lXxjUlwE&e=