Piraha return in wrong place
Issue #2257
closed
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
Comments (4)
-
-
- changed status to open
-
reporter - changed status to resolved
Change applied. Commit is 15f933249dc060ef833e62d2c1a3f385c8c461fa on https://bitbucket.org/cactuscode/cactus.git
-
- changed status to closed
- Log in to comment
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 isgr->group(1)
which has already been assigned toval
befor inval = meval(gr->group(1),eedata)
. This does not lead to incorrect results (sincemin(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
ideally the tests would also include a call of
min(42.)
ie a single argument to min or max.