- 
                Notifications
    You must be signed in to change notification settings 
- Fork 0
Cds 421 multi adaptor intersect constraints simple #300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Cds 421 multi adaptor intersect constraints simple #300
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two things to be checked:
- is the self.mapped_requestafter (the merge-back operation) in the same space as the constraints that are sent to the sub-adaptors? actually, what matters is if by the time the sub-adaptors do the constraints intersection the request and the constraints are in the same space (i.e. mapped or not mapped)
- would it be possible to avoid the merge-back operation, e.g. by deciding that the multi-adaptor won't split the request in any way (by not doing intersect_constraints, split-by-month or other splitting operations), but leave that to the sub-adaptors?
| 
 For the second part, we could avoid the merge-back operation, and just send the original request to the splitting. However, I think we still want to do the normalise_request (+ intersect_constraints) at the top level adaptor as that means we can reject InvalidRequests before they get onto a worker. The merging of requests could be useful for the cads-obs adaptor, so I was thinking about putting it in the general tools to be used there | 
| 
 For the first part, if I understood what you mean.... The request passing to sub-adaptors has not changed. The self.mapped_request we have in multi.py is in the top level adaptor, this is then split this into X requests, each of which is sent to it's own adaptor. The constraints handling has changed, and previously they were not in the same space, i.e. the constraints were only in the top-level adaptor but the intersection wsa in the sub-adaptor. Now we pass the constraints to the sub-adaptors, so when we intersect_constraints in the sub-adaptors we have both the request and constraints. | 
| 
 As discussed on Teams, this is a known bug/potential incompatibility (mentioned in PR description). I will open a new PR to address this specifically. | 
| Additionally, moved the merge_requests function to the hcube_tools module | 
This is to intersect_constraints with MultiAdaptor datasets.
Approach:
This duplication of intersection may be repetitive, but it prevents the possibility of creating a very large number of sub-adaptor requests which may make logging/debugging nasty.
NOTE: Leaving the possibility of mapping being applied at the parent adaptor, but this contradicts using the same constraints for both parent and child adaptors. We do not have any parent level mapping, so not a problem in practice, but for sure can enforce correct behaviour.