Commit be41108e authored by Matthieu Schaller's avatar Matthieu Schaller Committed by Peter W. Draper
Browse files

Aggregated sort arrays

parent e4fd03ab
......@@ -338,7 +338,7 @@ struct cell {
struct xpart *xparts;
/*! Pointer for the sorted indices. */
struct sort_entry *sort[13];
struct sort_entry *sort;
/*! Super cell, i.e. the highest-level parent cell that has a hydro
* pair/self tasks */
......@@ -441,6 +441,9 @@ struct cell {
/*! Bit-mask indicating the sorted directions */
uint16_t sorted;
/*! Bit-mask indicating the sorted directions */
uint16_t sort_allocated;
#ifdef SWIFT_DEBUG_CHECKS
/*! Last (integer) time the cell's sort arrays were updated. */
......@@ -606,7 +609,7 @@ struct cell {
float dx_max_sort_old;
/*! Pointer for the sorted indices. */
struct sort_entry *sort[13];
struct sort_entry *sort;
/*! Bit mask of sort directions that will be needed in the next timestep. */
uint16_t requires_sorts;
......@@ -614,6 +617,9 @@ struct cell {
/*! Bit-mask indicating the sorted directions */
uint16_t sorted;
/*! Bit-mask indicating the sorted directions */
uint16_t sort_allocated;
/*! Bit mask of sorts that need to be computed for this cell. */
uint16_t do_sort;
......@@ -1296,16 +1302,61 @@ __attribute__((always_inline)) INLINE static void cell_ensure_tagged(
* @param flags Cell flags.
*/
__attribute__((always_inline)) INLINE static void cell_malloc_hydro_sorts(
struct cell *c, int flags) {
struct cell *c, const int flags) {
const int count = c->hydro.count;
/* Note that sorts can be used by different tasks at the same time (but not
* on the same dimensions), so we need separate allocations per dimension. */
for (int j = 0; j < 13; j++) {
if ((flags & (1 << j)) && c->hydro.sort[j] == NULL) {
if ((c->hydro.sort[j] = (struct sort_entry *)swift_malloc(
"hydro.sort", sizeof(struct sort_entry) * (count + 1))) == NULL)
/* Have we already allocated something? */
if (c->hydro.sort != NULL) {
/* Start by counting how many dimensions we need
and how many we already have */
const int num_arrays_wanted =
intrinsics_popcount(c->hydro.sort_allocated | flags);
const int num_already_allocated =
intrinsics_popcount(c->hydro.sort_allocated);
/* Do we already have what we want? */
if (num_arrays_wanted == num_already_allocated) return;
/* Allocate memory for the new array */
struct sort_entry *new_array = NULL;
if ((new_array = (struct sort_entry *)swift_malloc(
"hydro.sort", sizeof(struct sort_entry) * num_arrays_wanted *
(count + 1))) == NULL)
error("Failed to allocate sort memory.");
/* Now, copy the already existing arrays */
int from = 0;
int to = 0;
for (int j = 0; j < 13; j++) {
if (c->hydro.sort_allocated & (1 << j)) {
memcpy(&new_array[to * (count + 1)], &c->hydro.sort[from * (count + 1)],
sizeof(struct sort_entry) * (count + 1));
++from;
++to;
} else if (flags & (1 << j)) {
++to;
c->hydro.sort_allocated |= (1 << j);
}
}
/* Swap the pointers */
swift_free("hydro.sort", c->hydro.sort);
c->hydro.sort = new_array;
} else {
c->hydro.sort_allocated = flags;
/* Start by counting how many dimensions we need */
const int num_arrays = intrinsics_popcount(flags);
/* If there is anything, allocate enough memory */
if (num_arrays) {
if ((c->hydro.sort = (struct sort_entry *)swift_malloc(
"hydro.sort",
sizeof(struct sort_entry) * num_arrays * (count + 1))) == NULL)
error("Failed to allocate sort memory.");
}
}
......@@ -1319,14 +1370,44 @@ __attribute__((always_inline)) INLINE static void cell_malloc_hydro_sorts(
__attribute__((always_inline)) INLINE static void cell_free_hydro_sorts(
struct cell *c) {
for (int i = 0; i < 13; i++) {
if (c->hydro.sort[i] != NULL) {
swift_free("hydro.sort", c->hydro.sort[i]);
c->hydro.sort[i] = NULL;
}
if (c->hydro.sort != NULL) {
swift_free("hydro.sort", c->hydro.sort);
c->hydro.sort = NULL;
c->hydro.sort_allocated = 0;
}
}
/**
* @brief Returns the array of sorted indices for the gas particles of a given
* cell along agiven direction.
*
* @param c The #cell.
* @param sid the direction id.
*/
__attribute__((always_inline)) INLINE static struct sort_entry *
cell_get_hydro_sorts(const struct cell *c, const int sid) {
#ifdef SWIFT_DEBUG_CHECKS
if (sid >= 13 || sid < 0) error("Invalid sid!");
if (!(c->hydro.sort_allocated & (1 << sid)))
error("Sort not allocated along direction %d", sid);
#endif
/* We need to find at what position in the meta-array of
sorts where the corresponding sid has been allocated since
there might be gaps as we only allocated the directions that
are in use.
We create a mask with all the bits before the sid's one set to 1
and apply it on the list of allocated directions. We then count
the number of bits that are in the results to obtain the position
of the correspondin sid in the meta-array */
const int j = intrinsics_popcount(c->hydro.sort_allocated & ((1 << sid) - 1));
/* Return the corresponding array */
return &c->hydro.sort[j * (c->hydro.count + 1)];
}
/**
* @brief Allocate stars sort memory for cell.
*
......@@ -1334,16 +1415,61 @@ __attribute__((always_inline)) INLINE static void cell_free_hydro_sorts(
* @param flags Cell flags.
*/
__attribute__((always_inline)) INLINE static void cell_malloc_stars_sorts(
struct cell *c, int flags) {
struct cell *c, const int flags) {
const int count = c->stars.count;
/* Note that sorts can be used by different tasks at the same time (but not
* on the same dimensions), so we need separate allocations per dimension. */
for (int j = 0; j < 13; j++) {
if ((flags & (1 << j)) && c->stars.sort[j] == NULL) {
if ((c->stars.sort[j] = (struct sort_entry *)swift_malloc(
"stars.sort", sizeof(struct sort_entry) * (count + 1))) == NULL)
/* Have we already allocated something? */
if (c->stars.sort != NULL) {
/* Start by counting how many dimensions we need
and how many we already have */
const int num_arrays_wanted =
intrinsics_popcount(c->stars.sort_allocated | flags);
const int num_already_allocated =
intrinsics_popcount(c->stars.sort_allocated);
/* Do we already have what we want? */
if (num_arrays_wanted == num_already_allocated) return;
/* Allocate memory for the new array */
struct sort_entry *new_array = NULL;
if ((new_array = (struct sort_entry *)swift_malloc(
"stars.sort", sizeof(struct sort_entry) * num_arrays_wanted *
(count + 1))) == NULL)
error("Failed to allocate sort memory.");
/* Now, copy the already existing arrays */
int from = 0;
int to = 0;
for (int j = 0; j < 13; j++) {
if (c->stars.sort_allocated & (1 << j)) {
memcpy(&new_array[to * (count + 1)], &c->stars.sort[from * (count + 1)],
sizeof(struct sort_entry) * (count + 1));
++from;
++to;
} else if (flags & (1 << j)) {
++to;
c->stars.sort_allocated |= (1 << j);
}
}
/* Swap the pointers */
swift_free("stars.sort", c->stars.sort);
c->stars.sort = new_array;
} else {
c->stars.sort_allocated = flags;
/* Start by counting how many dimensions we need */
const int num_arrays = intrinsics_popcount(flags);
/* If there is anything, allocate enough memory */
if (num_arrays) {
if ((c->stars.sort = (struct sort_entry *)swift_malloc(
"stars.sort",
sizeof(struct sort_entry) * num_arrays * (count + 1))) == NULL)
error("Failed to allocate sort memory.");
}
}
......@@ -1357,14 +1483,44 @@ __attribute__((always_inline)) INLINE static void cell_malloc_stars_sorts(
__attribute__((always_inline)) INLINE static void cell_free_stars_sorts(
struct cell *c) {
for (int i = 0; i < 13; i++) {
if (c->stars.sort[i] != NULL) {
swift_free("stars.sort", c->stars.sort[i]);
c->stars.sort[i] = NULL;
}
if (c->stars.sort != NULL) {
swift_free("stars.sort", c->stars.sort);
c->stars.sort = NULL;
c->stars.sort_allocated = 0;
}
}
/**
* @brief Returns the array of sorted indices for the star particles of a given
* cell along agiven direction.
*
* @param c The #cell.
* @param sid the direction id.
*/
__attribute__((always_inline)) INLINE static struct sort_entry *
cell_get_stars_sorts(const struct cell *c, const int sid) {
#ifdef SWIFT_DEBUG_CHECKS
if (sid >= 13 || sid < 0) error("Invalid sid!");
if (!(c->stars.sort_allocated & (1 << sid)))
error("Sort not allocated along direction %d", sid);
#endif
/* We need to find at what position in the meta-array of
sorts where the corresponding sid has been allocated since
there might be gaps as we only allocated the directions that
are in use.
We create a mask with all the bits before the sid's one set to 1
and apply it on the list of allocated directions. We then count
the number of bits that are in the results to obtain the position
of the correspondin sid in the meta-array */
const int j = intrinsics_popcount(c->stars.sort_allocated & ((1 << sid) - 1));
/* Return the corresponding array */
return &c->stars.sort[j * (c->stars.count + 1)];
}
/** Set the given flag for the given cell. */
__attribute__((always_inline)) INLINE static void cell_set_flag(struct cell *c,
uint32_t flag) {
......
......@@ -629,7 +629,7 @@ void DOPAIR_SUBSET(struct runner *r, struct cell *restrict ci,
const float H = cosmo->H;
/* Pick-out the sorted lists. */
const struct sort_entry *restrict sort_j = cj->hydro.sort[sid];
const struct sort_entry *sort_j = cell_get_hydro_sorts(cj, sid);
const float dxj = cj->hydro.dx_max_sort;
/* Parts are on the left? */
......@@ -943,8 +943,8 @@ void DOPAIR1(struct runner *r, struct cell *ci, struct cell *cj, const int sid,
for (int k = 0; k < 3; k++) rshift += shift[k] * runner_shift[sid][k];
/* Pick-out the sorted lists. */
const struct sort_entry *restrict sort_i = ci->hydro.sort[sid];
const struct sort_entry *restrict sort_j = cj->hydro.sort[sid];
const struct sort_entry *restrict sort_i = cell_get_hydro_sorts(ci, sid);
const struct sort_entry *restrict sort_j = cell_get_hydro_sorts(cj, sid);
#ifdef SWIFT_DEBUG_CHECKS
/* Some constants used to checks that the parts are in the right frame */
......@@ -1198,8 +1198,8 @@ void DOPAIR1_BRANCH(struct runner *r, struct cell *ci, struct cell *cj) {
#ifdef SWIFT_DEBUG_CHECKS
/* Pick-out the sorted lists. */
const struct sort_entry *restrict sort_i = ci->hydro.sort[sid];
const struct sort_entry *restrict sort_j = cj->hydro.sort[sid];
const struct sort_entry *restrict sort_i = cell_get_hydro_sorts(ci, sid);
const struct sort_entry *restrict sort_j = cell_get_hydro_sorts(cj, sid);
/* Check that the dx_max_sort values in the cell are indeed an upper
bound on particle movement. */
......@@ -1276,8 +1276,8 @@ void DOPAIR2(struct runner *r, struct cell *ci, struct cell *cj, const int sid,
for (int k = 0; k < 3; k++) rshift += shift[k] * runner_shift[sid][k];
/* Pick-out the sorted lists. */
struct sort_entry *restrict sort_i = ci->hydro.sort[sid];
struct sort_entry *restrict sort_j = cj->hydro.sort[sid];
struct sort_entry *restrict sort_i = cell_get_hydro_sorts(ci, sid);
struct sort_entry *restrict sort_j = cell_get_hydro_sorts(cj, sid);
#ifdef SWIFT_DEBUG_CHECKS
/* Some constants used to checks that the parts are in the right frame */
......@@ -1790,8 +1790,8 @@ void DOPAIR2_BRANCH(struct runner *r, struct cell *ci, struct cell *cj) {
#ifdef SWIFT_DEBUG_CHECKS
/* Pick-out the sorted lists. */
const struct sort_entry *restrict sort_i = ci->hydro.sort[sid];
const struct sort_entry *restrict sort_j = cj->hydro.sort[sid];
const struct sort_entry *restrict sort_i = cell_get_hydro_sorts(ci, sid);
const struct sort_entry *restrict sort_j = cell_get_hydro_sorts(cj, sid);
/* Check that the dx_max_sort values in the cell are indeed an upper
bound on particle movement. */
......
......@@ -243,8 +243,8 @@ void DOPAIR1(struct runner *r, struct cell *ci, struct cell *cj, const int sid,
for (int k = 0; k < 3; k++) rshift += shift[k] * runner_shift[sid][k];
/* Pick-out the sorted lists. */
const struct sort_entry *restrict sort_i = ci->hydro.sort[sid];
const struct sort_entry *restrict sort_j = cj->hydro.sort[sid];
const struct sort_entry *restrict sort_i = cell_get_hydro_sorts(ci, sid);
const struct sort_entry *restrict sort_j = cell_get_hydro_sorts(cj, sid);
#ifdef SWIFT_DEBUG_CHECKS
/* Some constants used to checks that the parts are in the right frame */
......@@ -482,8 +482,8 @@ void DOPAIR1_BRANCH(struct runner *r, struct cell *ci, struct cell *cj) {
#ifdef SWIFT_DEBUG_CHECKS
/* Pick-out the sorted lists. */
const struct sort_entry *restrict sort_i = ci->hydro.sort[sid];
const struct sort_entry *restrict sort_j = cj->hydro.sort[sid];
const struct sort_entry *restrict sort_i = cell_get_hydro_sorts(ci, sid);
const struct sort_entry *restrict sort_j = cell_get_hydro_sorts(cj, sid);
/* Check that the dx_max_sort values in the cell are indeed an upper
bound on particle movement. */
......
......@@ -272,8 +272,8 @@ void DO_SYM_PAIR1_STARS(struct runner *r, struct cell *ci, struct cell *cj,
if (do_ci_stars) {
/* Pick-out the sorted lists. */
const struct sort_entry *restrict sort_j = cj->hydro.sort[sid];
const struct sort_entry *restrict sort_i = ci->stars.sort[sid];
const struct sort_entry *restrict sort_j = cell_get_hydro_sorts(cj, sid);
const struct sort_entry *restrict sort_i = cell_get_stars_sorts(ci, sid);
#ifdef SWIFT_DEBUG_CHECKS
/* Some constants used to checks that the parts are in the right frame */
......@@ -403,8 +403,8 @@ void DO_SYM_PAIR1_STARS(struct runner *r, struct cell *ci, struct cell *cj,
if (do_cj_stars) {
/* Pick-out the sorted lists. */
const struct sort_entry *restrict sort_i = ci->hydro.sort[sid];
const struct sort_entry *restrict sort_j = cj->stars.sort[sid];
const struct sort_entry *restrict sort_i = cell_get_hydro_sorts(ci, sid);
const struct sort_entry *restrict sort_j = cell_get_stars_sorts(cj, sid);
#ifdef SWIFT_DEBUG_CHECKS
/* Some constants used to checks that the parts are in the right frame */
......@@ -588,7 +588,7 @@ void DOPAIR1_SUBSET_STARS(struct runner *r, struct cell *restrict ci,
if (count_j == 0) return;
/* Pick-out the sorted lists. */
const struct sort_entry *restrict sort_j = cj->hydro.sort[sid];
const struct sort_entry *restrict sort_j = cell_get_hydro_sorts(cj, sid);
const float dxj = cj->hydro.dx_max_sort;
/* Sparts are on the left? */
......@@ -1072,7 +1072,8 @@ void DOSELF1_BRANCH_STARS(struct runner *r, struct cell *c) {
#define RUNNER_CHECK_SORT(TYPE, PART, cj, ci, sid) \
({ \
const struct sort_entry *restrict sort_j = cj->TYPE.sort[sid]; \
const struct sort_entry *restrict sort_j = \
cell_get_##TYPE##_sorts(cj, sid); \
\
for (int pjd = 0; pjd < cj->TYPE.count; pjd++) { \
const struct PART *p = &cj->TYPE.parts[sort_j[pjd].i]; \
......
......@@ -1322,8 +1322,8 @@ void runner_dopair1_density_vec(struct runner *r, struct cell *ci,
for (int k = 0; k < 3; k++) rshift += shift[k] * runner_shift[sid][k];
/* Pick-out the sorted lists. */
const struct sort_entry *restrict sort_i = ci->hydro.sort[sid];
const struct sort_entry *restrict sort_j = cj->hydro.sort[sid];
const struct sort_entry *restrict sort_i = cell_get_hydro_sorts(ci, sid);
const struct sort_entry *restrict sort_j = cell_get_hydro_sorts(cj, sid);
/* Get some other useful values. */
const int count_i = ci->hydro.count;
......@@ -1728,7 +1728,7 @@ void runner_dopair_subset_density_vec(struct runner *r,
const int count_j = cj->hydro.count;
/* Pick-out the sorted lists. */
const struct sort_entry *restrict sort_j = cj->hydro.sort[sid];
const struct sort_entry *sort_j = cell_get_hydro_sorts(cj, sid);
const float dxj = cj->hydro.dx_max_sort;
/* Get both particle caches from the runner and re-allocate
......@@ -2077,8 +2077,8 @@ void runner_dopair2_force_vec(struct runner *r, struct cell *ci,
for (int k = 0; k < 3; k++) rshift += shift[k] * runner_shift[sid][k];
/* Pick-out the sorted lists. */
const struct sort_entry *restrict sort_i = ci->hydro.sort[sid];
const struct sort_entry *restrict sort_j = cj->hydro.sort[sid];
const struct sort_entry *restrict sort_i = cell_get_hydro_sorts(ci, sid);
const struct sort_entry *restrict sort_j = cell_get_hydro_sorts(cj, sid);
/* Get some other useful values. */
const int count_i = ci->hydro.count;
......
......@@ -297,7 +297,7 @@ void runner_do_hydro_sort(struct runner *r, struct cell *c, int flags,
for (int k = 0; k < 8; k++) {
inds[k] = k;
if (c->progeny[k] != NULL && c->progeny[k]->hydro.count > 0) {
fingers[k] = c->progeny[k]->hydro.sort[j];
fingers[k] = cell_get_hydro_sorts(c->progeny[k], j);
buff[k] = fingers[k]->d;
off[k] = off[k];
} else
......@@ -314,7 +314,7 @@ void runner_do_hydro_sort(struct runner *r, struct cell *c, int flags,
}
/* For each entry in the new sort list. */
struct sort_entry *finger = c->hydro.sort[j];
struct sort_entry *finger = cell_get_hydro_sorts(c, j);
for (int ind = 0; ind < count; ind++) {
/* Copy the minimum into the new sort array. */
......@@ -335,8 +335,10 @@ void runner_do_hydro_sort(struct runner *r, struct cell *c, int flags,
} /* Merge. */
/* Add a sentinel. */
c->hydro.sort[j][count].d = FLT_MAX;
c->hydro.sort[j][count].i = 0;
struct sort_entry *entries = cell_get_hydro_sorts(c, j);
entries[count].d = FLT_MAX;
entries[count].i = 0;
/* Mark as sorted. */
atomic_or(&c->hydro.sorted, 1 << j);
......@@ -372,19 +374,21 @@ void runner_do_hydro_sort(struct runner *r, struct cell *c, int flags,
const double px[3] = {parts[k].x[0], parts[k].x[1], parts[k].x[2]};
for (int j = 0; j < 13; j++)
if (flags & (1 << j)) {
c->hydro.sort[j][k].i = k;
c->hydro.sort[j][k].d = px[0] * runner_shift[j][0] +
px[1] * runner_shift[j][1] +
px[2] * runner_shift[j][2];
struct sort_entry *entries = cell_get_hydro_sorts(c, j);
entries[k].i = k;
entries[k].d = px[0] * runner_shift[j][0] +
px[1] * runner_shift[j][1] +
px[2] * runner_shift[j][2];
}
}
/* Add the sentinel and sort. */
for (int j = 0; j < 13; j++)
if (flags & (1 << j)) {
c->hydro.sort[j][count].d = FLT_MAX;
c->hydro.sort[j][count].i = 0;
runner_do_sort_ascending(c->hydro.sort[j], count);
struct sort_entry *entries = cell_get_hydro_sorts(c, j);
entries[count].d = FLT_MAX;
entries[count].i = 0;
runner_do_sort_ascending(entries, count);
atomic_or(&c->hydro.sorted, 1 << j);
}
}
......@@ -393,7 +397,7 @@ void runner_do_hydro_sort(struct runner *r, struct cell *c, int flags,
/* Verify the sorting. */
for (int j = 0; j < 13; j++) {
if (!(flags & (1 << j))) continue;
struct sort_entry *finger = c->hydro.sort[j];
struct sort_entry *finger = cell_get_hydro_sorts(c, j);
for (int k = 1; k < count; k++) {
if (finger[k].d < finger[k - 1].d)
error("Sorting failed, ascending array.");
......@@ -529,7 +533,7 @@ void runner_do_stars_sort(struct runner *r, struct cell *c, int flags,
for (int k = 0; k < 8; k++) {
inds[k] = k;
if (c->progeny[k] != NULL && c->progeny[k]->stars.count > 0) {
fingers[k] = c->progeny[k]->stars.sort[j];
fingers[k] = cell_get_stars_sorts(c->progeny[k], j);
buff[k] = fingers[k]->d;
off[k] = off[k];
} else
......@@ -546,7 +550,7 @@ void runner_do_stars_sort(struct runner *r, struct cell *c, int flags,
}
/* For each entry in the new sort list. */
struct sort_entry *finger = c->stars.sort[j];
struct sort_entry *finger = cell_get_stars_sorts(c, j);
for (int ind = 0; ind < count; ind++) {
/* Copy the minimum into the new sort array. */
......@@ -567,8 +571,9 @@ void runner_do_stars_sort(struct runner *r, struct cell *c, int flags,
} /* Merge. */
/* Add a sentinel. */
c->stars.sort[j][count].d = FLT_MAX;
c->stars.sort[j][count].i = 0;
struct sort_entry *entries = cell_get_stars_sorts(c, j);
entries[count].d = FLT_MAX;
entries[count].i = 0;
/* Mark as sorted. */
atomic_or(&c->stars.sorted, 1 << j);
......@@ -598,19 +603,21 @@ void runner_do_stars_sort(struct runner *r, struct cell *c, int flags,
const double px[3] = {sparts[k].x[0], sparts[k].x[1], sparts[k].x[2]};
for (int j = 0; j < 13; j++)
if (flags & (1 << j)) {
c->stars.sort[j][k].i = k;
c->stars.sort[j][k].d = px[0] * runner_shift[j][0] +
px[1] * runner_shift[j][1] +
px[2] * runner_shift[j][2];
struct sort_entry *entries = cell_get_stars_sorts(c, j);
entries[k].i = k;
entries[k].d = px[0] * runner_shift[j][0] +
px[1] * runner_shift[j][1] +
px[2] * runner_shift[j][2];
}
}
/* Add the sentinel and sort. */
for (int j = 0; j < 13; j++)
if (flags & (1 << j)) {
c->stars.sort[j][count].d = FLT_MAX;
c->stars.sort[j][count].i = 0;
runner_do_sort_ascending(c->stars.sort[j], count);
struct sort_entry *entries = cell_get_stars_sorts(c, j);
entries[count].d = FLT_MAX;
entries[count].i = 0;
runner_do_sort_ascending(entries, count);
atomic_or(&c->stars.sorted, 1 << j);
}
}
......@@ -619,7 +626,7 @@ void runner_do_stars_sort(struct runner *r, struct cell *c, int flags,
/* Verify the sorting. */
for (int j = 0; j < 13; j++) {
if (!(flags & (1 << j))) continue;
struct sort_entry *finger = c->stars.sort[j];
struct sort_entry *finger = cell_get_stars_sorts(c, j);
for (int k = 1; k < count; k++) {
if (finger[k].d < finger[k - 1].d)
error("Sorting failed, ascending array.");
......
......@@ -202,6 +202,7 @@ void space_rebuild_recycle_mapper(void *map_data, int num_elements,
c->stars.dx_max_sort = 0.f;
c->black_holes.dx_max_part = 0.f;
c->hydro.sorted = 0;
c->hydro.sort_allocated = 0;
c->stars.sorted = 0;
c->hydro.count = 0;
c->hydro.count_total = 0;
......
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