Piraha return in wrong place

[Steven R. Brandt]

Piraha has a return in the wrong place. This showed up when processing min/max functions. On further investigation, there was a loop bound error as well.

index 49b02c9..ee06fc3 100644
--- a/src/piraha/Call.cc
+++ b/src/piraha/Call.cc
@@ -405,7 +405,7 @@ smart_ptr<Value> meval(smart_ptr<Group> gr,ExpressionEvaluationData *eedata) {
                 std::string par = get_parfile();
                 CCTK_Error(gr->line(),par.c_str(),current_thorn.c_str(),msg.str().c_str());
             }
-            for (int i=1; i<=gr->groupCount(); i++) {
+            for (int i=1; i<gr->groupCount(); i++) {
                 smart_ptr<Value> val_next = meval(gr->group(i),eedata);
                 // Make sure all arguments are either integer or real
                 if (val_next->type != PIR_REAL && val_next->type != PIR_INT) {
@@ -437,8 +437,8 @@ smart_ptr<Value> meval(smart_ptr<Group> gr,ExpressionEvaluationData *eedata) {
                     std::string par = get_parfile();
                     CCTK_Error(gr->line(),par.c_str(),current_thorn.c_str(),"internal error");
                 }
-                return val;
             }
+            return val;
         }
         // From here on only functions that take exactly one argument: the majority.
         else if (gr->groupCount() != 2) {

‌

The following change to expresions.par in the cactustest thorn should be made to go with this ticket

index b30787a..7f3ea06 100644
--- a/TestPar/test/expressions.par
+++ b/TestPar/test/expressions.par
@@ -3,7 +3,7 @@ ActiveThorns = "TestPar"
 TestPar::out_dir = $parfile

 TestPar::int2[0] = -10
-TestPar::real1[0] = 1.
+TestPar::real1[0] = min(min(6.6,max(max(0.2,1.0),0.5)),7)
 TestPar::real2[0] = -1
 TestPar::bool1[0] = yes
 TestPar::bool2[0] = no

[Roland Haas]

Looks mostly fine with me. Please apply. An actual pull request would have been nice though since it would have made it easier to inspect the code and verify the code in context.

For example the lower bound of the loop is somewhat off since the loop starts from 1 so the first val_next inspected is gr->group(1) which has already been assigned to val befor in val = meval(gr->group(1),eedata). This does not lead to incorrect results (since min(a,a) = a) but is a bit odd.

The test change is good to apply as well because, while it does remove a test for assigning a simple float to a parameter, there is another such assignment in

TestPar::real1[2] = 42.

ideally the tests would also include a call of min(42.) ie a single argument to min or max.

[Steven R. Brandt]

Change applied. Commit is 15f933249dc060ef833e62d2c1a3f385c8c461fa on https://bitbucket.org/cactuscode/cactus.git