As a developer, I want to understand why there are two identical requests to obtain location information from WRDS and, if the second call is unnecessary, remove it #283
Comments
|
Original Redmine Comment Speculation in #96392-70. First thing I would check is whether the same thing happens without the feature group, i.e. add them as singletons - I suspect it does (and hence is unrelated to feature groups). |
|
Original Redmine Comment James: Posting the below since I wrote it while you wrote your message. I'll try the singleton run in a bit. I want to spin up another run of a feature group with thresholds, first. Here is what I wrote... I ran the evaluation in debug for #96392 and noted these log messages: I wasn't sure why there were two instants of "[WALN6, FROV2]" in the "These need lookups" list. I also posted additional logging below. Later, James wondered if this is expected:
James also recommended,
I'll do that in the future. For now, I'm focused on testing the feature group capability with thresholds. Leaving this ticket, Hank ========================================================================== |
|
Original Redmine Comment Right you are, James. Its not related to feature groups. If I remove the group and include just the two features FROV2 and WALN6, I see this in the logs: So there is some redundancy in the calls, regardless. It would be good to remove the duplicate call to the service, but, again, not a high priority. Description and subject modified. Hank |
|
Original Redmine Comment Right. I'm not certain it is redundancy, actually, because it is requesting for two separate pairs of feature dimensions, even though one of them is fixed (nws) and hence the logging appears to show duplicate requests (edit: the @INFO@ logging could probably be a bit clearer about that, so I suppose that is one possible outcome of this ticket). They are not, actually, duplicate requests (edit: I mean from the perspective of wres - literally speaking, they are), but I'm not sure why they need to be requested as pairs, rather than tuples. Perhaps that is just how the feature service works. |
|
Original Redmine Comment In summary, needs input from Jesse. There may be scope to deduplicate here, minimally to clarify the log messages, but the reason for the apparently duplicate requests is that two pairs of feature dimensions are being resolved, both with the same (known) feature dimension hook of @nws_lid@, so it isn't a blatant bug, it is more complicated than that. edit: it would also be worth checking w/ 5.13 - same behavior? I assume, yes. |
|
Original Redmine Comment Confirmed in -dev it has the two identical requests to WRDS (-dev has the WRES 5.12) so this is pre-existing. I see from your debug log a reason (not an excuse): The API between @featurefinder@ and @WrdsFeatureService@ has a single @from@ and a single @to@: So the same "from" was used twice but not the same "to." It is an opportunity for deduplication of requests. I guess you would want to make the bulkLookup accept a @list@ and then further complicate the @featurefinder@ to somehow take advantage of that. But herein lies a question about whether we want the opportunity for these kinds of optimizations. The assumption of "one feature name per every other feature name for this evaluation" that @featurefinder@ currently has is the very assumption that allows for these kinds of optimizations. If you remove that assumption, I think you might have to remove the existing optimizations and it might avoid this one. You would have to instead pick an order whereby features could snowball, like "first look up left dimension to right dimension, second look up left dimension to baseline dimension, third look up right dimension to baseline dimension, fourth look up baseline dimension to left dimension, fifth, look up.." and then pick a number of iterations after which you stop, because the first lookup can go from 1 to N, the second lookup can go from 1 to N, then the third lookup can go from 1 to N, etc. Edit: or I guess you would just have a lookup or two for each declared feature instead of attempting to look at many features from one dimension to another. |
|
Original Redmine Comment Jesse, I don't have time to think about this deeply, but, in general, this duplication does not appear to be a big thing. Thus, if it complicates the code to get rid of it, its probably not worth it. Rejection is perfectly reasonable for this ticket. Thanks, Hank |
|
Original Redmine Comment This is kind of what I wondered, the optimization might not be worth it even if it is literally a duplicate request. I suppose you could still implement optimization above the caller by caching responses, edit: per url request, that is (there is no reason that a response would change during an evaluation and, strictly, that would render the evaluation invalid anyway). Still, even though that is not an api change, it is also extra complexity. |
Author Name: Hank (Hank)
Original Redmine Issue: 98035, https://vlab.noaa.gov/redmine/issues/98035
Original Date: 2021-10-27
The declaration is below. The log messages indicating two calls:
I don't think two calls should be necessary, but it doesn't do any harm from what I can tell, so this is a normal priority item.
Thanks,
Hank
EDIT: If you remove the @FeatureGroup@, below, and, instead, just evaluation WALN6 and FROV2 as singletons, the duplicate calls are still seen. So its not a feature group byproduct.
===================================================
The text was updated successfully, but these errors were encountered: