FreeFem / FreeFem-sources

FreeFEM source code
https://freefem.org/
Other
746 stars 187 forks source link

Missing subtraction assignment operator (-=) #268

Closed cmd8 closed 1 year ago

cmd8 commented 1 year ago

When working with real[int] evaluated from a varf, there is an addition assignment operator (+=), but not a subtraction assignment operator.

real[int] Vec = vA(0, Vh); // vA is a varf and Vh is a fespace
Vec += vA(0, Vh); // this works
Vec -= vA(0, Vh); // this fails
prj- commented 1 year ago

Do you feel like making a PR?

cmd8 commented 1 year ago

I would be happy to, but I wasn't sure where this would appear? Is it in https://github.com/FreeFem/FreeFem-sources/blob/1312e9d9c06fec8d3d49f1452df6f951901d85c9/src/fflib/Operator.hpp#L338?

prj- commented 1 year ago

Actually, I'd say it's around there https://github.com/FreeFem/FreeFem-sources/blob/develop/src/fflib/lgfem.cpp#L6557-L6589.

cmd8 commented 1 year ago

Thanks for pointing me that way. It looks like I can clone this code block and create :

  TheOperators->Add(
    "-=",
...
);

Am I correct in understanding that this would also require something like RNM_VirtualMatrix< K >::minusAx and RNM_VirtualMatrix< K >::minusAtx? Sorry for the novice questions.

prj- commented 1 year ago

I would not worry about that and simply add the relevant code (so all the OpArraytoLinearForm instantiations).

cmd8 commented 1 year ago

I'm still missing something. This seems like the way to go, but involves adding a minusAx to the RNM_VirtualMatrix class. How could we achieve this with only instantiations of OpArraytoLinearForm?

TheOperators->Add( "-=",
  new OneBinaryOperator< set_eq_array_sub< KN_< double >, RNM_VirtualMatrix< double >::minusAx > >,
  new OneBinaryOperator< set_eq_array_sub< KN_< double >, RNM_VirtualMatrix< double >::minusAtx > >,
  new OpArraytoLinearForm< double, Mesh, v_fes >(atype< KN_< double > >( ), false, false, false),
  new OpArraytoLinearForm< double, Mesh3, v_fes3 >(atype< KN_< double > >( ), false, false, false), // 3D volume
  new OpArraytoLinearForm< double, MeshS, v_fesS >(atype< KN_< double > >( ), false, false, false), // 3D surface
  new OpArraytoLinearForm< double, MeshL, v_fesL >(atype< KN_< double > >( ), false, false, false)  // 3D surface
);
prj- commented 1 year ago

Why do you need

  new OneBinaryOperator< set_eq_array_sub< KN_< double >, RNM_VirtualMatrix< double >::minusAx > >,
  new OneBinaryOperator< set_eq_array_sub< KN_< double >, RNM_VirtualMatrix< double >::minusAtx > >,
cmd8 commented 1 year ago

Because, as far as I can tell, the -= operator does not do anything if I don't provide this. If I comment out those lines, I find different results when I evaluate the -= operator:

real[int] vec1 = vA(0, Vh);
vec1 -= vA(0, Vh); //gives the incorrect result.

vec1 = vA(0, Vh);
real[int] vec2 = vA(0, Vh);
vec1 -= vec2; // gives the correct result
prj- commented 1 year ago

Well, if you didn’t add the proper code along those lines, of course it won’t work, it will just return += disguised as -=. Did you add the rest of the code to handle the minus sign?

cmd8 commented 1 year ago

Right, of course. I thought you were suggesting that this could somehow be done with only OpArraytoLinearForm instantiations.

cmd8 commented 1 year ago

I'm not having any success. I don't know enough about these templates and classes to make this work. Unless you already have a quick solution in mind, I suggest this issue be closed. It is not an essential functionality.