lattice / quda

QUDA is a library for performing calculations in lattice QCD on GPUs.
https://lattice.github.io/quda
Other
289 stars 97 forks source link

HeavyQuark Residual use in MILC interface #138

Closed mathiaswagner closed 9 years ago

mathiaswagner commented 10 years ago

For qudaMultishiftInvert there is a line in milc_interface.cpp

invertParam.residual_type = (target_fermilab_residual[0] != 0) ? QUDA_HEAVY_QUARK_RESIDUAL : QUDA_L2_RELATIVE_RESIDUAL;

while for qudaInvert it is

invertParam.residual_type = (target_residual != 0) ? QUDA_L2_RELATIVE_RESIDUAL : QUDA_HEAVY_QUARK_RESIDUAL;

It does not matter if only one residual is specified but probably it would be better to use a consistent convention. Just the question which is the preferred one.

maddyscientist commented 10 years ago

I guess we should check for the heavy quark residual being non-zero, since this is the non-standard case. Given the MILC convention of using “0” to disable a residual type, perhaps it would make sense to check that neither are non-zero as a failsafe (and give an error if they are both zero).


This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by

reply email and destroy all copies of the original message.

mathiaswagner commented 10 years ago

Yes, that is probably right. Anyhow, I think when MILC specifies both it wants both to residuals to be satisfied and QUDA currently only does one or the other. Correct me if I am wrong there.

On 19.06.2014, at 12:50, mikeaclark notifications@github.com wrote:

I guess we should check for the heavy quark residual being non-zero, since this is the non-standard case. Given the MILC convention of using “0” to disable a residual type, perhaps it would make sense to check that neither are non-zero as a failsafe (and give an error if they are both zero).


This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by

reply email and destroy all copies of the original message.

— Reply to this email directly or view it on GitHub.


Mathias Wagner Department of Physics SW 117 - Indiana University Bloomington, IN 47405 email: mathwagn@indiana.edu

maddyscientist commented 10 years ago

You’re wrong :) QUDA will do up three residuals simultanesouly (L2 RELATIVE, L2 ABSOLUTE and HEAVY_QUARK). You set the flag using bitwise OR and a final cast.

From: Mathias Wagner notifications@github.com<mailto:notifications@github.com> Reply-To: lattice/quda reply@reply.github.com<mailto:reply@reply.github.com> Date: Thursday, June 19, 2014 at 12:53 PM To: lattice/quda quda@noreply.github.com<mailto:quda@noreply.github.com> Cc: M Clark mclark@nvidia.com<mailto:mclark@nvidia.com> Subject: Re: [quda] HeavyQuark Residual use in MILC interface (#138)

Yes, that is probably right. Anyhow, I think when MILC specifies both it wants both to residuals to be satisfied and QUDA currently only does one or the other. Correct me if I am wrong there.

On 19.06.2014, at 12:50, mikeaclark notifications@github.com<mailto:notifications@github.com> wrote:

I guess we should check for the heavy quark residual being non-zero, since this is the non-standard case. Given the MILC convention of using “0” to disable a residual type, perhaps it would make sense to check that neither are non-zero as a failsafe (and give an error if they are both zero).


This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by

reply email and destroy all copies of the original message.

— Reply to this email directly or view it on GitHub.


Mathias Wagner Department of Physics SW 117 - Indiana University Bloomington, IN 47405 email: mathwagn@indiana.edumailto:mathwagn@indiana.edu

— Reply to this email directly or view it on GitHubhttps://github.com/lattice/quda/issues/138#issuecomment-46586766.

mathiaswagner commented 10 years ago

OK. I guess then just the MILC interface needs to be checked and fixed to set this correctly. As I am anyhow working on heavy-quarks I'll do that.

mathiaswagner commented 10 years ago

updated milc interface in mw/milc_invert branch in commit 314661225505f20361dbfd35c790b427bdee842f

I am not quite sure about the initialization of invertParam.residual_type in milc_interface.cpp:755

maddyscientist commented 10 years ago

Can you be more clear: what do you mean by you’re not sure by the initialization?


This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by

reply email and destroy all copies of the original message.

mathiaswagner commented 10 years ago

To add the wanted residuals using bitwise or I have to start from some zero value. Then I can use a bitwise or the set the corresponding bit for L2 and heavy-quark.

invertParam.residual_type = static_cast<QudaResidualType_s>(0); invertParam.residual_type = (target_residual[0] != 0) ? static_cast<QudaResidualType_s> ( invertParam.residual_type | QUDA_L2_RELATIVE_RESIDUAL) : invertParam.residual_type; invertParam.residual_type = (target_fermilab_residual[0] != 0) ? static_cast<QudaResidualType_s> (invertParam.residual_type | QUDA_HEAVY_QUARK_RESIDUAL) : invertParam.residual_type;

I explicitly set invertParam.residual to zero so that I can set the desired bits. I did not find whether invertParam.residual_type is initialized to a specific value but it assumed it is QUDA_INVALID_RESIDUAL which comes down to QUDA_INVALID_ENUM and that is defined as INT_MIN and if I remember correctly this is not zero.

What I would prefer is QUDA_INVALID_RESIDUAL=0 so that if both target_residuals are zero (i.e. no residual set at all) the residual_type is zero. In my current implementation I should add a check for residual_type==0 and then set it to QUDA_INVALID_RESIDUAL.

maddyscientist commented 10 years ago

It is set in check_param.h

ifdef INIT_PARAM

P(residual_type, QUDA_L2_RELATIVE_RESIDUAL);

else

P(residual_type, QUDA_INVALID_RESIDUAL);

endif

This means it is initialized to be QUDA_L2_RELATIVE_RESIDUAL, and when do a parameter check, we check that it is not QUDA_INVALID_RESIDUAL. Since the residual_type was added after the interface was defined, I made the default L2 relative so that it wouldn't break someone who had interfaced into QUDA prior to its introduction.

For the moment, I think I'm going to suggest that you just set the residual_type to 0 like you are doing rather than relying on it initialization. If we changed it like you say, then it runs the risk of breaking someone's code.

At some point I will do a full audit of the interface, which will likely lead to the interface breaking for applications and require an update. That's not now though.

Sound ok to you?

mathiaswagner commented 10 years ago

Fine with me. I will merge the interface changes in to 0.7 and that will close this issue.

maddyscientist commented 10 years ago

Can this be closed now?