Modify

Opened 3 months ago

Last modified 2 months ago

#2195 confirmed defect

Multipole Thorn: Why is max_vars set to 10?

Reported by: zachetie@… Owned by:
Priority: minor Milestone:
Component: Other Version: development version
Keywords: Cc:

Description

arrangements/EinsteinAnalysis/Multipole/src/multipole.cc , line 22 reads:

static const int max_vars = 10;

Occasionally I'd like to output a multipole decomposition, e.g., for 12 variables. When I attempt this, because of the above limit, I get the mysterious error message that appears due to the assert on line 42 of this same file.

First, I think this should be a freely specifiable parameter.

Second, the error message should be clearer about what is wrong.

Third, by just setting the above to 30, I was able to perform a run with 12 output variables. Is this the correct fix? (The run no longer errors out.)

Attachments (0)

Change History (1)

comment:1 Changed 2 months ago by Roland Haas

Priority: unsetminor
Status: newconfirmed

There should certainly be a nice error message if the user specified list of variables is longer than the maximum hard-coded in the source file.

I assume the error that you saw as the assert:

assert(vs->n_vars < max_vars); // Too many variables in the variables list

which at least in the source code shows what went wrong. Certainly not the nicest way of handling an error that can be triggered by user inpu.

This can be changed easily to something like:

if(vs->n_vars >= max_vars)
  CCTK_VERROR("More than %d variables used in %s::variables parameter.", CCTK_THORNSTRING, max_vars);

for a nicer error message (since it contains the allowed maximum value).

As far as making that a runtime parameter goes, you can certainly give it a try and see what is required to make max_vars a CCTK_INT parameter, then propose a pull request. My guess is that technically you are leaving what C++ allows you to do, since at least "old" versions of C++ do not allow things like:

void foo(int n) {
  int array[n];
}

even if C99 does allow this but most compilers are fine with this. You may have to use std::vector to achieve the same effect in this case though since max_vars is used to size a static variable in Multipole_Calc.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as confirmed The ticket will remain with no owner.
Next status will be 'review'.
as The resolution will be set.
to The owner will be changed from (none) to the specified user.
The owner will be changed from (none) to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.