Closed jacobpwagner closed 3 years ago
@jacobpwagner is this still an open issue? Just going through old stuff to see where to focus.
Yeah, I believe it is still open. It's probably a pretty quick fix, though. It must have just slipped through the cracks.
Here, I think it's just finding the global minimum:
This is still not fixed. It seems to still be gating around the global min, even on the latest github version:
@Pandapip1 , rather than being fully implemented, the option was simply removed in 956808f71b5befa23688552351e2b25b091e7c82, so I think this is expected behavior.
In constraining the search for the gate boundary returned from
gate_mindensity
orgate_mindensity2
, themin
,max
, andgate_range
args work as expected. However, thepivot
option does not. According to the docs:"pivot | logical value. If TRUE, we choose as the two peaks the largest peak and its neighboring peak. See details."
(details from
mindensity
): "In the default case, the two peaks of interest are the two largest peaks obtained from the density function. However, if pivot is TRUE, we choose the largest peak and its neighboring peak as the two peaks of interest. In this case, the neighboring peak is the peak immediately to the left of the largest peak if positive is TRUE. Otherwise, the neighboring peak is selected as the peak to the right"However, this does not seem to work at all and a quick glance at the source suggests that the option is never utilized. Demonstration:
Here, I think it's just finding the global minimum:
The same call with
pivot=FALSE
results in the exact same result. Of course, usinggate_range
ormin
/max
, we can get the desired boundary.I'm not sure if the option was just never implemented, but we should probably either implement it fully (it would be kind of handy in automated pipelines) or remove it from the args and docs.