#1505: remove the MoL number of variables accumulators ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: reviewed_ok Priority: optional | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: MoL ------------------------------------+--------------------------------------- Changes (by eschnett):
* status: review => reviewed_ok
Comment:
Patch 01 looks good.
Patch 02 looks good.
mol.patch looks almost fine: - There is a stray "printf" that probably wants to go away. - It contains malloc/realloc calls that may look nicer in C++ (std::vector!), or that could be simplified in C: Instead of {{{ EvolvedVariableIndex = (CCTK_INT *)malloc(sizeof(CCTK_INT)*newSize); }}} write {{{ EvolvedVariableIndex = malloc(newsize * sizeof *EvolvedVariableIndex); }}} (no need to cast the malloc return value; no need to remember the type of EvolvedVariableIndex). That's mostly a question of preference, of course.
Patch 03: - Why is LinearCombination renamed? Aliased functions are type-safe, so all callers will notice. - Consider using CCTK_VError for errors (not important).
With these caveats: Please apply, then announce on mailing list.
Question: Should the accumulator parameter be removed, or at least its documentation changed and marked obsolete? Maybe output a paramcheck warning if it is set?