Let's look at this, here's the new code:
HessianNorms(invU, cdepth);
double skin = cdepth[0]/gpu->sim.xcells - gpu->sim.cut;
double yskin = cdepth[1]/gpu->sim.ycells - gpu->sim.cut;
double zskin = cdepth[2]/gpu->sim.zcells - gpu->sim.cut;
The array cdepth is filed with the new a, b, and c by means of an external
call. But as far as I know, all unit cells are orthorhombic, so we've
always been able to use:
// Recalculate a, b, c
double a =
pNTPData->ucell[0];
double b =
sqrt(pNTPData->ucell[1] * pNTPData->ucell[1] +
pNTPData->ucell[4] * pNTPData->ucell[4]);
double c =
sqrt(pNTPData->ucell[2] * pNTPData->ucell[2] +
pNTPData->ucell[5] * pNTPData->ucell[5] +
pNTPData->ucell[8] * pNTPData->ucell[8]);
because of the constraints that imposes on the column space of the unit
cell matrix.
So either the first code clause is overkill and repeated work or we have to
revisit the previous orthorhombic assumption. Which is it?
But even then, why would one change the unit cell matrix without updating
the a, b, and c derived from it? And yes, I'm impatient w/r to fixing bugs.
Guilty.
On Fri, Mar 18, 2022 at 10:42 AM Scott Le Grand <varelse2005.gmail.com>
wrote:
> If the unit cell changes, wouldn't a, b, and c change as well since they
> are derived from said unit cell?
>
> As for the speed of response. If it's important to anyone, they'll
> respond. If not, well they can make their principled stand against me and
> my impatience later. Otherwise, I acknowledge your point but also my
> okeydokey here, nobody is debugging this thing. It's an easy fix. And I
> need to do that fix to make it exploit the fixed point local coordinate
> system, but I could leave the seemingly sloppy incorrect behavior here.
> Driveby contributions to PMEMD like this however are just not good IMO and
> why I threw my hands up in frustration after trying to refactor for
> performance in 2020. It's a Sisyphusian effort and the tsunami of
> unattended warning messages and dead code in the thing alone is enough to
> make one run screaming into the night IMO. It was a single unitialized
> variable that broke PMEMD for Silicon Therapeutics in 2020. But also, why
> is there is so much dead code?
>
> The author of this code added a new and completely superfluous method of
> recalculating a, b,and c without exploiting the properties of the
> orthorhombic coordinate system to do it more simply as it is done in
> gpu_pressure_scale, which itself does the same thing internally as
> gpu_update_box if there's an MC barostat. And I hope you can forgive me for
> being annoyed at the needless repetition of functionality here w/o any
> notes as to why this was necessary.
>
> On Fri, Mar 18, 2022 at 10:22 AM David A Case <david.case.rutgers.edu>
> wrote:
>
>> On Fri, Mar 18, 2022, Scott Le Grand wrote:
>>
>> >Okeydokey, nobody cares. Therefore it's a bug and I'm going to fix it.
>> Test
>> >will be updated to reflect this. What SM revision should I use for the
>> test
>> >reference files?
>>
>> ??!? How fast do you expect people to respond? I looked at this and
>> didn't
>> see any obvious error -- the two routines are called at different places:
>> gpu_pressure_scale only when ntp>0 and barostat=1, whereas gpu_update_box
>> is only called when re-weighting trajectories.
>>
>> But I don't know the code here. Clearly, you must be convinced that a,b,c
>> need to be updated when reweight is > 0. I'll try to look at what
>> behavior
>> changes when you make the change. But I'm travelling to the ACS meeting
>> tomorrow, so you may disappointed if you require immediate feedback.
>>
>> ....dac
>>
>>
>> _______________________________________________
>> AMBER-Developers mailing list
>> AMBER-Developers.ambermd.org
>> http://lists.ambermd.org/mailman/listinfo/amber-developers
>>
>
_______________________________________________
AMBER-Developers mailing list
AMBER-Developers.ambermd.org
http://lists.ambermd.org/mailman/listinfo/amber-developers
Received on Fri Mar 18 2022 - 11:30:02 PDT
