Commit 601ca4fe authored by iker_martin's avatar iker_martin
Browse files

Anadidos comentarios a los codigos para una mejor comprension

parent 4d03d3a9
......@@ -7,9 +7,17 @@
#include "ini.h"
void malloc_config_arrays(configuration *user_config, int resizes);
void def_struct_config_file(configuration *config_file, MPI_Datatype *config_type);
void def_struct_config_file_array(configuration *config_file, MPI_Datatype *config_type);
/*
* Funcion utilizada para leer el fichero de configuracion
* y guardarlo en una estructura para utilizarlo en el futuro.
*
* Primero lee la seccion "general" y a continuacion cada una
* de las secciones "resize%d".
*/
static int handler(void* user, const char* section, const char* name,
const char* value) {
configuration* pconfig = (configuration*)user;
......@@ -58,6 +66,13 @@ static int handler(void* user, const char* section, const char* name,
return 1;
}
/*
* Crea y devuelve una estructura de configuracion a traves
* de un nombre de fichero dado.
*
* La memoria de la estructura se reserva en la funcion y es conveniente
* liberarla con la funcion "free_config()"
*/
configuration *read_ini_file(char *file_name) {
configuration *config = NULL;
......@@ -78,7 +93,13 @@ configuration *read_ini_file(char *file_name) {
/*
* Reserva de memoria para los vectores de la estructura de configuracion
*
* Si se llama desde fuera de este codigo, tiene que reservarse la estructura
* Si se llama desde fuera de este fichero, la memoria de la estructura
* tiene que reservarse con la siguiente linea:
* "configuration *config = malloc(sizeof(configuration));"
*
* Sin embargo se puede obtener a traves de las funciones
* - read_ini_file
* - recv_config_file
*/
void malloc_config_arrays(configuration *user_config, int resizes) {
if(user_config != NULL) {
......@@ -89,6 +110,9 @@ void malloc_config_arrays(configuration *user_config, int resizes) {
}
}
/*
* Libera toda la memoria de una estructura de configuracion
*/
void free_config(configuration *user_config) {
if(user_config != NULL) {
free(user_config->iters);
......@@ -100,6 +124,10 @@ void free_config(configuration *user_config) {
}
}
/*
* Imprime por salida estandar toda la informacion que contiene
* la configuracion pasada como argumento
*/
void print_config(configuration *user_config) {
if(user_config != NULL) {
int i;
......@@ -114,48 +142,84 @@ void print_config(configuration *user_config) {
//
//
//
//
//
//||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| \\
//||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ||
//| FUNCIONES DE INTERCOMUNICACION DE ESTRUCTURA DE CONFIGURACION ||
//||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ||
//||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| //
/*
* Envia una estructura de configuracion al grupo de procesos al que se
* enlaza este grupo a traves del intercomunicador pasado como argumento.
*
* Esta funcion tiene que ser llamada por todos los procesos del mismo grupo
* e indicar cual es el proceso raiz que se encargara de enviar la
* configuracion al otro grupo.
*/
void send_config_file(configuration *config_file, int root, MPI_Comm intercomm) {
MPI_Datatype config_type, config_type_array;
// Obtener un tipo derivado para enviar todos los
// datos escalares con una sola comunicacion
def_struct_config_file(config_file, &config_type);
MPI_Bcast(config_file, 1, config_type, root, intercomm);
// Obtener un tipo derivado para enviar los tres vectores
// de enteros con una sola comunicacion
def_struct_config_file_array(config_file, &config_type_array);
MPI_Bcast(config_file, 1, config_type, root, intercomm);
MPI_Bcast(config_file, 1, config_type_array, root, intercomm);
MPI_Bcast(config_file->factors, config_file->resizes, MPI_FLOAT, root, intercomm);
//Liberar tipos derivados
MPI_Type_free(&config_type);
MPI_Type_free(&config_type_array);
}
/*
* Recibe una estructura de configuracion desde otro grupo de procesos
* y la devuelve. La memoria de la estructura se reserva en esta funcion.
*
* Esta funcion tiene que ser llamada por todos los procesos del mismo grupo
* e indicar cual es el proceso raiz del otro grupo que se encarga de enviar
* la configuracion a este grupo.
*
* La memoria de la configuracion devuelta tiene que ser liberada con
* la funcion "free_config".
*/
configuration *recv_config_file(int root, MPI_Comm intercomm) {
MPI_Datatype config_type, config_type_array;
configuration *config_file = malloc(sizeof(configuration) * 1);
def_struct_config_file(config_file, &config_type);
// Obtener un tipo derivado para recibir todos los
// datos escalares con una sola comunicacion
def_struct_config_file(config_file, &config_type);
MPI_Bcast(config_file, 1, config_type, root, intercomm);
malloc_config_arrays(config_file, config_file->resizes);
// Obtener un tipo derivado para enviar los tres vectores
// de enteros con una sola comunicacion
malloc_config_arrays(config_file, config_file->resizes); // Reserva de memoria de los vectores
def_struct_config_file_array(config_file, &config_type_array);
MPI_Bcast(config_file, 1, config_type_array, root, intercomm);
MPI_Bcast(config_file->factors, config_file->resizes, MPI_FLOAT, root, intercomm);
//Liberar tipos derivados
MPI_Type_free(&config_type);
MPI_Type_free(&config_type_array);
return config_file;
}
/*
* Tipo derivado para enviar 6 elementos especificos
* de la estructura de configuracion con una sola comunicacion.
*/
void def_struct_config_file(configuration *config_file, MPI_Datatype *config_type) {
int i, counts = 6;
int blocklengths[6] = {1, 1, 1, 1, 1, 1};
......@@ -166,7 +230,7 @@ void def_struct_config_file(configuration *config_file, MPI_Datatype *config_typ
types[0] = types[1] = types[2] = types[3] = types[4] = MPI_INT;
types[5] = MPI_FLOAT;
//Rellenar vector displs
// Rellenar vector displs
MPI_Get_address(config_file, &dir);
MPI_Get_address(&(config_file->resizes), &displs[0]);
......@@ -182,6 +246,10 @@ void def_struct_config_file(configuration *config_file, MPI_Datatype *config_typ
MPI_Type_commit(config_type);
}
/*
* Tipo derivado para enviar tres vectores de enteros
* de la estructura de configuracion con una sola comunicacion.
*/
void def_struct_config_file_array(configuration *config_file, MPI_Datatype *config_type) {
int i, counts = 3;
int blocklengths[3] = {1, 1, 1};
......@@ -203,7 +271,9 @@ void def_struct_config_file_array(configuration *config_file, MPI_Datatype *conf
for(i=0;i<counts;i++) displs[i] -= dir;
// Tipo derivado para enviar un solo elemento de tres vectores
MPI_Type_create_struct(counts, blocklengths, displs, types, &aux);
// Tipo derivado para enviar N elementos de tres vectores(3N en total)
MPI_Type_create_resized(aux, 0, 1*sizeof(int), config_type);
MPI_Type_commit(config_type);
}
......@@ -15,15 +15,10 @@ typedef struct
} configuration;
configuration *read_ini_file(char *file_name);
void malloc_config_arrays(configuration *user_config, int resizes);
void free_config(configuration *user_config);
void print_config(configuration *user_config);
// MPI Intercomm functions
void send_config_file(configuration *config_file, int root, MPI_Comm intercomm);
configuration *recv_config_file(int root, MPI_Comm intercomm);
......@@ -44,16 +44,13 @@ int main(int argc, char *argv[]) {
group->grp = 0;
group->argv = argv;
MPI_Comm_get_parent(&(group->parents));
if(group->parents != MPI_COMM_NULL ) { // Si son procesos hijos deben recoger la distribucion
if(group->parents != MPI_COMM_NULL ) { // Si son procesos hijos deben comunicarse con las padres
Sons_init();
} else {
} else { // Si son el primer grupo de procesos, recogen la configuracion inicial
config_file = read_ini_file(argv[1]);
if(config_file->sdr > 0) {
malloc_comm_array(&(group->sync_array), config_file->sdr , group->myId, group->numP);
printf("Vector reservado por padres\n");
fflush(stdout);
}
}
......@@ -75,7 +72,15 @@ int main(int argc, char *argv[]) {
}
/*
* Bucle de computo principal
* Función de trabajo principal.
*
* Incializa los datos para realizar el computo y a continuacion
* pasa a realizar "maxiter" iteraciones de computo.
*
* Terminadas las iteraciones realiza el redimensionado de procesos.
* Si el redimensionado se realiza de forma asincrona se
* siguen realizando iteraciones de computo hasta que termine la
* comunicacion asincrona y realizar entonces la sincrona.
*/
int work() {
int iter, maxiter;
......@@ -90,9 +95,33 @@ int work() {
checkpoint(iter);
/*
iter = 0
while(maxiter) { //FIXME AÑADIR VALOR
iterate(matrix, config_file->matrix_tam);
iter++;
//check_async(iter);
}
*/
return 0;
}
/*
* Se realiza el redimensionado de procesos por parte de los padres.
*
* Se crean los nuevos procesos con la distribucion fisica elegida y
* a continuacion se transmite la informacion a los mismos.
*
* Si hay datos asincronos a transmitir, primero se comienza a
* transmitir estos y se termina la funcion. Se tiene que comprobar con
* la funcion "??" que se han terminado de enviar //TODO
*
* Si hay ademas datos sincronos a enviar, no se envian aun.
*
* Si solo hay datos sincronos se envian tras la creacion de los procesos
* y finalmente se desconectan los dos grupos de procesos.
*/
int checkpoint(int iter) {
// Comprobar si se tiene que realizar un redimensionado
......@@ -121,6 +150,9 @@ int checkpoint(int iter) {
return 1;
}
/*
* Se encarga de realizar la creacion de los procesos hijos.
*/
void TC(int numS){
// Inicialización de la comunicación con SLURM
int dist = config_file->phy_dist[group->grp +1];
......@@ -134,7 +166,12 @@ void TC(int numS){
}
}
/*
* Inicializacion de los datos de los hijos.
* En la misma se reciben datos de los padres: La configuracion
* de la ejecucion a realizar; y los datos a recibir de los padres
* ya sea de forma sincrona, asincrona o ambas.
*/
void Sons_init() {
// Enviar a los hijos que grupo de procesos son
......@@ -145,9 +182,8 @@ void Sons_init() {
config_file = recv_config_file(ROOT, group->parents);
int numP_parents = config_file->procs[group->grp -1];
if(config_file->sdr > 0) {
if(config_file->sdr > 0) { // Recibir datos sincronos
recv_sync(&(group->sync_array), config_file->sdr, group->myId, group->numP, ROOT, group->parents, numP_parents);
group->sync_array = malloc(5);
}
// Desconectar intercomunicador con los hijos
......@@ -164,6 +200,7 @@ void Sons_init() {
/*
* Simula la ejecucción de una iteración de computo en la aplicación
* que dura al menos un tiempo de "time" segundos.
*/
void iterate(double *matrix, int n) {
double start_time, actual_time;
......
#!/bin/bash
#SBATCH -N 1
#SBATCH -N 2
#module load gcc/6.4.0
#module load openmpi/1.10.7
......
......@@ -32,6 +32,12 @@ void getIds_intercomm(struct Dist_data dist_data, int numP_other, int **idS);
void mallocCounts(struct Counts *counts, int numP);
void freeCounts(struct Counts *counts);
/*
* Realiza un envio síncrono del vector array desde este grupo de procesos al grupo
* enlazado por el intercomunicador intercomm.
*
* El vector array no se modifica en esta funcion.
*/
int send_sync(char *array, int qty, int myId, int numP, int root, MPI_Comm intercomm, int numP_child) {
int rootBcast = MPI_PROC_NULL;
int *idS = NULL;
......@@ -48,9 +54,6 @@ int send_sync(char *array, int qty, int myId, int numP, int root, MPI_Comm inter
getIds_intercomm(dist_data, numP_child, &idS); // Obtener rango de Id hijos a los que este proceso manda datos
printf("-1!! -- Vector de padres realizan COUNTS\n");
fflush(stdout);
MPI_Barrier(MPI_COMM_WORLD);
send_sync_arrays(dist_data, array, rootBcast, numP_child, idS[0], idS[1], counts.counts, counts.zero_arr, counts.displs, counts.zero_arr);
freeCounts(&counts);
......@@ -60,19 +63,21 @@ int send_sync(char *array, int qty, int myId, int numP, int root, MPI_Comm inter
}
/*
* Realiza una recepcion síncrona del vector array a este grupo de procesos desde el grupo
* enlazado por el intercomunicador intercomm.
*
* El vector array se reserva dentro de la funcion y se devuelve en el mismo argumento.
* Tiene que ser liberado posteriormente por el usuario.
*/
void recv_sync(char **array, int qty, int myId, int numP, int root, MPI_Comm intercomm, int numP_parents) {
int *idS = NULL;
struct Counts counts;
struct Dist_data dist_data;
printf("Vector de hijos mandan datos\n");
fflush(stdout);
MPI_Barrier(MPI_COMM_WORLD);
// Obtener distribución para este hijo
get_dist(qty, myId, numP, &dist_data);
//*array = malloc(dist_data.tamBl * sizeof(char));
*array = malloc(qty * sizeof(char));
*array = malloc(dist_data.tamBl * sizeof(char));
dist_data.intercomm = intercomm;
/* PREPARAR DATOS DE RECEPCION SOBRE VECTOR*/
......@@ -86,18 +91,23 @@ void recv_sync(char **array, int qty, int myId, int numP, int root, MPI_Comm int
free(idS);
}
/*
* Reserva memoria para un vector de hasta "qty" elementos.
* Los "qty" elementos se disitribuyen entre los "numP" procesos
* que llaman a esta funcion.
*/
void malloc_comm_array(char **array, int qty, int myId, int numP) {
struct Dist_data dist_data;
get_dist(qty, myId, numP, &dist_data);
//*array = malloc(dist_data.tamBl * sizeof(char));
*array = malloc(qty * sizeof(char));
*array = malloc(dist_data.tamBl * sizeof(char));
}
/*
* Send to children Compute_data arrays which change in each iteration
* Envia a los hijos un vector que es redistribuido a los procesos
* hijos. Antes de realizar la comunicacion, cada proceso padre calcula sobre que procesos
* del otro grupo se transmiten elementos.
*/
void send_sync_arrays(struct Dist_data dist_data, char *array, int rootBcast, int numP_child, int idI, int idE,
int *sendcounts, int *recvcounts, int *sdispls, int *rdispls) {
......@@ -117,29 +127,25 @@ void send_sync_arrays(struct Dist_data dist_data, char *array, int rootBcast, in
// MPI_Alltoallv(array, sendcounts, sdispls, MPI_CHAR, NULL, recvcounts, rdispls, MPI_CHAR, dist_data.intercomm);
}
/*
* Recibe de los padres un vector que es redistribuido a los procesos
* de este grupo. Antes de realizar la comunicacion cada hijo calcula sobre que procesos
* del otro grupo se transmiten elementos.
*/
void recv_sync_arrays(struct Dist_data dist_data, char *array, int root, int numP_parents, int idI, int idE,
int *sendcounts, int *recvcounts,int *sdispls, int *rdispls) {
int i;
char *aux;
printf("A -- Vector de hijos realizan COUNTS\n");
fflush(stdout);
MPI_Barrier(MPI_COMM_WORLD);
// Ajustar los valores de recepcion
if(idI == 0) {
set_counts(0, numP_parents, dist_data, recvcounts);
idI++;
}
printf("B -- Vector de hijos realizan COUNTS\n");
fflush(stdout);
MPI_Barrier(MPI_COMM_WORLD);
for(i=idI; i<idE; i++) {
set_counts(i, numP_parents, dist_data, recvcounts);
rdispls[i] = rdispls[i-1] + recvcounts[i-1];
}
printf("C -- Vector de hijos realizan COUNTS\n");
fflush(stdout);
MPI_Barrier(MPI_COMM_WORLD);
// print_counts(*dist_data, recvcounts, rdispls, numP_parents, "Recv");
/* COMUNICACION DE DATOS */
......@@ -186,8 +192,8 @@ void get_dist(int qty, int id, int numP, struct Dist_data *dist_data) {
/*
* Obtains for a given process Id, how many elements will
* send or recieve from the process indicated in Dist_data
* Obtiene para el Id de un proceso dado, cuantos elementos
* enviara o recibira desde el proceso indicado en Dist_data.
*/
void set_counts(int id, int numP, struct Dist_data data_dist, int *sendcounts) {
struct Dist_data other;
......@@ -228,29 +234,48 @@ void getIds_intercomm(struct Dist_data dist_data, int numP_other, int **idS) {
int idI, idE;
int tamOther = dist_data.qty / numP_other;
int remOther = dist_data.qty % numP_other;
// Indica el punto de corte del grupo de procesos externo que
// divide entre los procesos que tienen
// un tamaño tamOther + 1 y un tamaño tamOther
int middle = (tamOther + 1) * remOther;
if(middle > dist_data.ini) { // First subgroup
// Calcular idI teniendo en cuenta si se comunica con un
// proceso con tamano tamOther o tamOther+1
if(middle > dist_data.ini) { // First subgroup (tamOther+1)
idI = dist_data.ini / (tamOther + 1);
} else { // Second subgroup
} else { // Second subgroup (tamOther)
idI = ((dist_data.ini - middle) / tamOther) + remOther;
}
if(middle >= dist_data.fin) { // First subgroup
// Calcular idR teniendo en cuenta si se comunica con un
// proceso con tamano tamOther o tamOther+1
if(middle >= dist_data.fin) { // First subgroup (tamOther +1)
idE = dist_data.fin / (tamOther + 1);
idE = (dist_data.fin % (tamOther + 1) > 0 && idE+1 <= numP_other) ? idE+1 : idE;
} else { // Second subgroup
} else { // Second subgroup (tamOther)
idE = ((dist_data.fin - middle) / tamOther) + remOther;
idE = ((dist_data.fin - middle) % tamOther > 0 && idE+1 <= numP_other) ? idE+1 : idE;
}
//free(*idS);
idS = malloc(2 * sizeof(int));
(*idS)[0] = idI;
(*idS)[1] = idE;
}
/*
* Reserva memoria para los vectores de counts/displs de la funcion
* MPI_Alltoallv. Todos los vectores tienen un tamaño de numP, que es la
* cantidad de procesos en el otro grupo de procesos.
*
* El vector counts indica cuantos elementos se comunican desde este proceso
* al proceso "i" del otro grupo.
*
* El vector displs indica los desplazamientos necesarios para cada comunicacion
* con el proceso "i" del otro grupo.
*
* El vector zero_arr se utiliza cuando se quiere indicar un vector incializado
* a 0 en todos sus elementos. Sirve para indicar que no hay comunicacion.
*/
void mallocCounts(struct Counts *counts, int numP) {
counts->counts = calloc(numP, sizeof(int));
if(counts->counts == NULL) { MPI_Abort(MPI_COMM_WORLD, -2);}
......@@ -262,6 +287,12 @@ void mallocCounts(struct Counts *counts, int numP) {
if(counts->zero_arr == NULL) { MPI_Abort(MPI_COMM_WORLD, -2);}
}
/*
* Libera la memoria interna de una estructura Counts.
*
* No libera la memoria de la estructura counts si se ha alojado
* de forma dinamica.
*/
void freeCounts(struct Counts *counts) {
free(counts->counts);
free(counts->displs);
......
......@@ -43,6 +43,19 @@ void print_Info(MPI_Info info);
//--------------PUBLIC FUNCTIONS---------------//
/*
* Se solicita la creacion de un nuevo grupo de "numP" procesos con una distribucion
* fisica "type_dist".
*
* Se puede solicitar en primer plano, encargandose por tanto el proceso que llama a esta funcion,
* o en segundo plano, donde un hilo se encarga de configurar esta creacion.
*
* Si se pide en primer plano, al terminarla es posible llamar a "check_slurm_comm()" para crear
* los procesos.
*
* Si se pide en segundo plano, llamar a "check_slurm_comm()" comprobara si la configuracion para
* crearlos esta lista, y si es asi, los crea.
*/
int init_slurm_comm(char **argv, int myId, int numP, int root, int type_dist, int type_creation) {
slurm_data = malloc(sizeof(struct Slurm_data));
......@@ -74,6 +87,10 @@ int init_slurm_comm(char **argv, int myId, int numP, int root, int type_dist, in
return 0;
}
/*
* Comprueba si una configuracion para crear un nuevo grupo de procesos esta lista,
* y en caso de que lo este, se crea un nuevo grupo de procesos con esa configuracion.
*/
int check_slurm_comm(int myId, int root, MPI_Comm comm, MPI_Comm *child) {
int spawn_err = COMM_IN_PROGRESS;
......@@ -96,6 +113,14 @@ int check_slurm_comm(int myId, int root, MPI_Comm comm, MPI_Comm *child) {
}
//--------------PRIVATE SPAWN TYPE FUNCTIONS---------------//
/*
* Funcion llamada por un hilo para que este se encarge
* de configurar la creacion de un nuevo grupo de procesos.
*
* Una vez esta lista la configuracion y es posible crear los procesos
* se avisa al hilo maestro.
*/
void* thread_work(void* creation_data_arg) {
struct Creation_data *creation_data = (struct Creation_data*) creation_data_arg;
......@@ -108,6 +133,11 @@ void* thread_work(void* creation_data_arg) {
//--------------PRIVATE SPAWN CREATION FUNCTIONS---------------//
/*
* Configura la creacion de un nuevo grupo de procesos, reservando la memoria
* para una llamada a MPI_Comm_spawn, obteniendo una distribucion fisica
* para los procesos y creando un fichero hostfile.
*/
void processes_dist(char *argv[], int numP_childs, int type) {
int jobId, ptr;
char *tmp;
......@@ -146,7 +176,10 @@ void processes_dist(char *argv[], int numP_childs, int type) {
slurm_free_job_info_msg(j_info);
}
/*
* Crea un grupo de procesos segun la configuracion indicada por la funcion
* "processes_dist()".
*/
int create_processes(int myId, int root, MPI_Comm *child, MPI_Comm comm) {
int spawn_err = MPI_Comm_spawn(slurm_data->cmd, MPI_ARGV_NULL, slurm_data->qty_procs, slurm_data->info, root, comm, child, MPI_ERRCODES_IGNORE);
......@@ -162,6 +195,18 @@ int create_processes(int myId, int root, MPI_Comm *child, MPI_Comm comm) {
return spawn_err;
}
/*
* Obtiene la distribucion fisica del grupo de procesos a crear, devolviendo
* cuantos nodos se van a utilizar y la cantidad de procesos que alojara cada
* nodo.
*
* Se permiten dos tipos de distribuciones fisicas segun el valor de "type":
*
* COMM_PHY_NODES (1): Orientada a equilibrar el numero de procesos entre
* todos los nodos disponibles.
* COMM_PHY_CPU (2): Orientada a completar la capacidad de un nodo antes de
* ocupar otro nodo.
*/
void node_dist(slurm_job_info_t job_record, int type, int total_procs, int **qty, int *used_nodes) {
int i, asigCores;
int tamBl, remainder;
......@@ -206,6 +251,16 @@ void node_dist(slurm_job_info_t job_record, int type, int total_procs, int **qty
free(procs);
}
/*
* Crea un fichero que se utilizara como hostfile
* para un nuevo grupo de procesos.
*
* El nombre es devuelto en el argumento "file_name",
* que tiene que ser un puntero vacio.
*
* Ademas se devuelve un descriptor de fichero para
* modificar el fichero.
*/
int create_hostfile(char *jobId, char **file_name) {
int ptr, err, len;
......@@ -223,6 +278,11 @@ int create_hostfile(char *jobId, char **file_name) {
return ptr; // Devolver puntero a fichero
}
/*
* Rellena un fichero hostfile indicado por ptr con los nombres
* de los nodos a utilizar indicados por "job_record" y la cantidad
* de procesos que alojara cada nodo indicado por "qty".
*/
void fill_hostfile(slurm_job_info_t job_record, int ptr, int *qty, int used_nodes) {
int i=0;
char *host;
......@@ -235,9 +295,14 @@ void fill_hostfile(slurm_job_info_t job_record, int ptr, int *qty, int used_node
free(host);
}
slurm_hostlist_destroy(hostlist);
}
/*
* Escribe en el fichero hostfile indicado por ptr una nueva linea.
*
* Esta linea indica el nombre de un nodo y la cantidad de procesos a
* alojar en ese nodo.
*/
int write_hostfile_node(int ptr, int qty, char *node_name) {
int err, len_node, len_int, len;
char *line;
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment