Skip to content

Conversation

@waynemitchell
Copy link
Contributor

This is work from @dinesh-parthasarathy implementing flexible AMG cycles in which a user may define arbitrary order for visiting levels of the hierarchy as well as arbitrary relaxation methods and weights each time a level is visited. This capability may be combined with genetic programming to find optimized cycle structures that provide superior performance compared to traditional V-cycles.

Parthasarathy and others added 30 commits June 13, 2023 16:57
add member variables and implement getters, setters, accessor functions to:
- define arbitrary cycling structure i.e. sequence of inter-grid operations.
- define relaxation type at each cycling node.
- define relaxation weights at each cycling node.
- define ::wnumber of relaxation sweeps at each cycling node.
- define struct config that stores the cycle structure input variables
- parse input arrays from command line, "," to separate elements in an array, "|" to separate two input arrays
- parse multiple arguments for multiple configs
- setup the grid hierarchy only once, and perform multiple solves in loop for multiple configurations.
- wrap the config loop to set amg inputs around solver_id=1, PCG
- fix: calculate initial relative residual norm for convergence rate calculation
- print convergence rate only if amg cycle structure is defined by user inputs
*----------------------------------------------------------------------------------------*/

HYPRE_Int
HYPRE_BoomerAMGFlexibleSetCycleStruct( HYPRE_Solver solver,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would prefer the naming convention, HYPRE_BoomerAMGSetFlexibleCycleStruct(), etc. throughout this PR. Thoughts from others?


while ( cycle_struct_flexible[i] == 0 || // same level
cycle_struct_flexible[i] == 1 || // cycle up
cycle_struct_flexible[i] == -1 ) // cycle down
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this right, you are basically relying on the loop going off the end of the allocated cycle_struct_flexible array, which is not a safe way of doing things. You could either have the user explicitly set the length of the cycle structure or have something like the last entry in cycle_struct_flexible be a specified termination character.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to keep the user inputs to a minimum, so I don't expect the length to be set explicitly. But the last entry of the list supplied would be a hint to terminate the loop (for example -2).

{
hypre_TFree((AMGhybrid_data -> cycle_struct_flexible), HYPRE_MEMORY_HOST);
}
(AMGhybrid_data -> cycle_struct_flexible) = cycle_struct_flexible;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to free these in the destructor for AMGhybrid_data as well.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the destructor for BoomerAMG called within hypre_AMGHybridDestroy should release memory allocated for flexible parameter arrays.

if (pcg_precond) { hypre_BoomerAMGDestroy(pcg_precond);

is_flexible[k] = 1;
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The additional is_flexible array seems unnecessary to me? Couldn't we just check level < num_levels_flexible in the main cycling loop?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree! I have removed the additional is_flexible array (9559077).

HYPRE_Int *cycle_struct_flexible, *relax_types_flexible, *relax_orders_flexible;
HYPRE_Real *outer_weights_flexible, *relax_weights_flexible, *cgc_scaling_factors_flexible;
HYPRE_Real cgc_scaling_factor_tmp;
HYPRE_Int index;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe name index as flexible_index or something in order to distinguish it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been renamed (9559077).

src/test/ij.c Outdated
HYPRE_Int P_max_elmts = 4;
HYPRE_Int cycle_type;
/* flexible cycling params*/
HYPRE_Int flexible_cycle = 0, length_cycle_flexible = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think we need the extra flexible_cycle flag.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been removed (01c7b7f).

src/test/ij.c Outdated
build_matrix_type = -1;
build_matrix_arg_index = arg_index;
}
else if ( strcmp(argv[arg_index], "-flexamg") == 0 )
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that it is required to specify all the different flexible cycle parameter arrays together here if using the -flexamg option. Is there value in allowing the user to specify only a subset of the options?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have made some modifications. The user must supply the values for cycle_struct_flexible to enable flexible cycling. The other arrays are optional only if you want to do something different at each step of the cycling. Else, the BoomerAMG parameters set using the other non-flexible interfaces is simply used for flexible cycling (01c7b7f).

relax_order = relax_orders_flexible[index];
relax_weight[level] = relax_weights_flexible[index];
omega[level] = outer_weights_flexible[index];
num_sweep = 1;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an implicit requirement here that the user needs to set all of these parameter arrays if using flexible cycles (i.e. they cannot just set a subset). We should either make this requirement more explicit with some error checking, or relax it by allowing default values (e.g. if the user set relax_type_flexible but not relax_order_flexible, just use a default value for relax_order, etc.).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated such that the user can supply only a subset (9559077).

1. rename index to index_flex
2. remove is_flexible array flag
3. allow users to pass only a subset of flexible cycle parameters
1. remove flexible_cycle flag
2. allow user to only specify a subset of the flexible options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants