Solving the problem with threshold_density option in associndex - #62
Conversation
Cambios en la función associndex y funciones que la llaman para que los cálculos por defecto no incluyan un threshold.
Updated functions called by "associndex" to set the default threshold to NULL
Updated so that, by default, there is not a threshold density. I have added an option threshold_density = "Weibull".
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the handling of the threshold_density parameter across several functions, changing the default value to NULL and introducing a "Weibull" option for outlier-based filtering. The review feedback correctly identifies a logic issue where setting the threshold to the maximum observed density would inadvertently filter out that specific data point; using Inf is recommended to ensure no interactions are excluded when the threshold is disabled. Additionally, the reviewer provided constructive feedback regarding code style improvements, specifically addressing inconsistent indentation and the standardization of assignment operators and spacing for better readability.
| if(is.null(threshold_density)){ | ||
| threshold_density = K$limit[[2]] | ||
| message("Based on Weibull distribution fitted to the observed values, the threshold density has been set to: ", threshold_density, ".") | ||
| # To effectively avoid the use of a threshold, we set its value to the maximum | ||
| # observed density. | ||
| threshold_density = max(y) | ||
| } |
There was a problem hiding this comment.
Hola, al establecer threshold_density = max(y), las interacciones con la densidad máxima serán filtradas debido a que la condición de filtrado es un menor estricto (<). Para desactivar el umbral de forma efectiva, como se indica en la documentación, sería mejor usar Inf. Esto asegurará que no se filtre ninguna interacción cuando threshold_density es NULL. También he aprovechado para mejorar el estilo del bloque.
if (is.null(threshold_density)) {
# To effectively avoid the use of a threshold, we set its value to infinity
# so no interactions are filtered out.
threshold_density <- Inf
}| int_data_0<-comm_to_RN_UNI(int_data, cover_data) | ||
| db_inter_0 <- pre_associndex_UNISITE_UNI(int_data_0) | ||
| db_inter_0$Dcr <- db_inter_0$Fcr/db_inter_0$Ac | ||
| db_inter_0$Dro <- db_inter_0$Fro/db_inter_0$Ao | ||
| # Dro has repeated values for each recruit species. The next line removes repeated | ||
| # values. | ||
| unique_Dro <- stats::aggregate(Dro ~ Recruit, data = db_inter_0[, c("Recruit", "Dro")], mean) | ||
| y <- c(db_inter_0$Dcr,unique_Dro$Dro) |
There was a problem hiding this comment.
Hola, la indentación en este bloque es inconsistente. Para mejorar la legibilidad, sería bueno ajustarla a 2 espacios. También he añadido espacios alrededor de los operadores (/, ~) y he homogeneizado el uso de <- para las asignaciones, siguiendo las guías de estilo comunes en R.
int_data_0 <- comm_to_RN_UNI(int_data, cover_data)
db_inter_0 <- pre_associndex_UNISITE_UNI(int_data_0)
db_inter_0$Dcr <- db_inter_0$Fcr / db_inter_0$Ac
db_inter_0$Dro <- db_inter_0$Fro / db_inter_0$Ao
# Dro has repeated values for each recruit species. The next line removes repeated
# values.
unique_Dro <- stats::aggregate(Dro ~ Recruit, data = db_inter_0[, c("Recruit", "Dro")], mean)
y <- c(db_inter_0$Dcr, unique_Dro$Dro)| if(threshold_density == "Weibull"){ | ||
|
|
||
| # Suggest a threshold value based on recruitment density outliers according | ||
| # to a Weibull distribution. Uses "extremevalues" package. | ||
| # Zero densities are common, so we will check only for suspiciously high densities. | ||
| y <- y[which(y>0)] | ||
| K <- extremevalues::getOutliers(y, method = "I", distribution = "weibull", rho=c(1,1)) # We | ||
| #consider that a value is an outlier if it is above the limit where less | ||
| #than 1 observation is expected. | ||
|
|
||
| threshold_density = K$limit[[2]] | ||
| n_excluded <- length(K$iRight) | ||
| message("Based on Weibull distribution fitted to the observed values, the threshold density has been set to: ", threshold_density, ". ", n_excluded, " cases were excluded.") | ||
| } |
There was a problem hiding this comment.
Hola, en este bloque también hay algunas inconsistencias de estilo. Para mejorar la legibilidad, sugiero ajustar la indentación, usar <- para las asignaciones y añadir espacios alrededor de los operadores. Te dejo una sugerencia con estos cambios.
if (threshold_density == "Weibull") {
# Suggest a threshold value based on recruitment density outliers according
# to a Weibull distribution. Uses "extremevalues" package.
# Zero densities are common, so we will check only for suspiciously high densities.
y <- y[which(y > 0)]
K <- extremevalues::getOutliers(y, method = "I", distribution = "weibull", rho = c(1, 1)) # We
# consider that a value is an outlier if it is above the limit where less
# than 1 observation is expected.
threshold_density <- K$limit[[2]]
n_excluded <- length(K$iRight)
message("Based on Weibull distribution fitted to the observed values, the threshold density has been set to: ", threshold_density, ". ", n_excluded, " cases were excluded.")
}
|
No he aplicado la sugerencia de que solo se eliminen posibles outlayers si corresponden a canopies con muy baja cobertura porque no veo cómo definir, con los datos disponibles, cuando debe considerarse que una cobertura es, per se, demasiado baja para el cálculo de los índices de asociación. De todas formas, será muy raro que un outlayer de densidad se deba a una especie con alta cobertura, porque tendría que tener auténticos céspedes de reclutas bajo la copa de todos los ejemplares presentes en el muestreo. |
|
Julio, ¿no podríamos detectar coberturas extremadamente bajas con cualquier método de detección de outliers? |
|
Lo he intentado usando extremeValues en varias redes pero, o no salen outlayers o solo aparecen para coberturas altas. Esto creo que se debe a que las distribuciones suelen estar dominadas por los valores bajos (tipo log-normal). |
|
Sí, la verdad las coberturas de las canopy vistas en un boxplot se apelotonan en la parte inferior, por lo que es difícil encontrar outliers en esa parte. Pero, por otra parte, tenemos la sensación de que Weibull es demasiado alegre eliminando densidades altas. ¿No habría manera de hacer que Weibull sea un poco más restrictivo? |
|
No lo comenté antes, pero también he resuelto el problema que comentaste de la repetición de valores de densidad en open: ahora cada valor aparece solo una vez. Esto debería echar la distribución más hacia la derecha (porque en open suele haber densidades muy bajas y muchos ceros), lo que normalmente debería aumentar el threshold y reducir el número de outlayers. Creo que aún no habéis podido probar la nueva versión de associndex porque me parece que Paco no ha aceptado todavía los cambios (a lo mejor es porque se me ha pasado revisar los tests de la función y por eso salta el mensaje de "All checks have failed"). A ver si ahora os parece menos restrictiva. |
|
Hemos revisado la nueva función de associndex. En esta versión, Julio ha cambiado el threshold por defecto para que no aplique ningún filtro, y ha añadido como opciones una distribución de Weibull o una densidad de reclutas definida por el usuario. Nuestra conclusión es que no conviene aplicar ningún filtro automáticamente. associndex debería limitarse a informar sobre la cobertura de la nodriza y la frecuencia de los reclutas con las que se ha calculado cada interacción, dejando al usuario la decisión sobre qué filtrar. Esta decisión se basa en dos observaciones. Por un lado, no estamos detectando coberturas bajas como outliers, que era una condición necesaria junto con una alta frecuencia de reclutas para eliminar una interacción. Por otro lado, tampoco queremos que Weibull elimine interacciones con alta frecuencia de reclutas, ya que consideramos que esas observaciones son válidas. Si os parece bien, modificamos Alicia y yo associndex y las funciones que dependen de el. |
|
Hola Por mí suena bien. Puedo hacer merge de este pull request de Julio y vosotros ya partís de ahí (i.e. hacéis nuevo fork y pull request). O hacéis fork del fork de Julio y seguís editando ahí. Imagino que la primera opción es más fácil ¿no? |
|
Gracias, Paco. Efectivamente, la primera opción es la más sencilla. |
|
¡Hecho! Ya podéis hacer fork y seguir modificando |
3 participants
He hecho los cambios que sugieren Ali y Miguel. Ahora la función no aplica ningún threshold a menos que se le indique explícitamente con un número o usando la distribución Weibull.