mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-07-12 13:29:04 +00:00
Merge pull request #1513 from Hestia-Homes/fix/widen-cohort-search-v2
Widen EPC-prediction cohort search one step when the normal radius is too sparse
This commit is contained in:
commit
bd3ff8fcdc
3 changed files with 127 additions and 8 deletions
|
|
@ -471,7 +471,13 @@ def handler(
|
|||
overrides_reader = InMemoryPropertyOverridesReader(overrides_by_pid)
|
||||
prediction_attrs_reader = OverrideBackedPredictionAttributesReader(overrides_reader)
|
||||
comparables_repo = EpcComparablePropertiesRepository(
|
||||
epc_client, geospatial, nearby_postcodes=PostcodesIoClient()
|
||||
epc_client,
|
||||
geospatial,
|
||||
nearby_postcodes=PostcodesIoClient(),
|
||||
# One step wider than the default 1000m/30 reach, tried only when that
|
||||
# normal radius doesn't surface enough same-type comparables (ADR-0034
|
||||
# amendment) — a single extra step outward, not an unbounded expansion.
|
||||
widen_nearby_postcodes=PostcodesIoClient(radius_m=3000, limit=60),
|
||||
)
|
||||
predictor = EpcPrediction()
|
||||
|
||||
|
|
|
|||
|
|
@ -75,10 +75,17 @@ class EpcComparablePropertiesRepository(ComparablePropertiesRepository):
|
|||
epc_client: CohortEpcClient,
|
||||
geospatial: CohortGeospatial,
|
||||
nearby_postcodes: Optional[NearbyPostcodes] = None,
|
||||
widen_nearby_postcodes: Optional[NearbyPostcodes] = None,
|
||||
) -> None:
|
||||
self._epc_client = epc_client
|
||||
self._geospatial = geospatial
|
||||
self._nearby_postcodes = nearby_postcodes
|
||||
# A second, wider-reaching `NearbyPostcodes` source, consulted only when
|
||||
# the normal-radius walk fails to reach `minimum` same-type matches — one
|
||||
# extra step outward before genuinely giving up (ADR-0034 amendment). A
|
||||
# single configured step today; the same pattern chains to further steps
|
||||
# later without changing this class's contract.
|
||||
self._widen_nearby_postcodes = widen_nearby_postcodes
|
||||
# Cohort certs skipped because they are not yet mappable. Accumulates
|
||||
# across every postcode the instance serves; the caller reads it after
|
||||
# the run to report the mapper gaps (see modelling_e2e handler).
|
||||
|
|
@ -117,12 +124,41 @@ class EpcComparablePropertiesRepository(ComparablePropertiesRepository):
|
|||
remaining, further-away postcodes are not fetched, so a dense area
|
||||
resolves in one or two searches instead of the whole radius. Without a
|
||||
configured ``NearbyPostcodes`` source this degrades to the seed postcode
|
||||
alone."""
|
||||
postcodes = (
|
||||
self._nearby_postcodes.nearby(postcode, coordinates)
|
||||
if self._nearby_postcodes is not None
|
||||
else [postcode]
|
||||
)
|
||||
alone.
|
||||
|
||||
If the normal-radius walk still falls short of ``minimum`` matches and a
|
||||
``widen_nearby_postcodes`` source is configured, the walk is retried
|
||||
once against that wider source before giving up — a single extra step
|
||||
outward, not an unbounded expansion."""
|
||||
postcodes = self._nearby(self._nearby_postcodes, postcode, coordinates)
|
||||
candidates, matches = self._walk(postcodes, enough, minimum)
|
||||
|
||||
if (
|
||||
enough is not None
|
||||
and matches < minimum
|
||||
and self._widen_nearby_postcodes is not None
|
||||
):
|
||||
wider_postcodes = self._nearby(
|
||||
self._widen_nearby_postcodes, postcode, coordinates
|
||||
)
|
||||
candidates, _ = self._walk(wider_postcodes, enough, minimum)
|
||||
|
||||
return candidates
|
||||
|
||||
def _nearby(
|
||||
self,
|
||||
source: Optional[NearbyPostcodes],
|
||||
postcode: str,
|
||||
coordinates: Optional[Coordinates],
|
||||
) -> list[str]:
|
||||
return source.nearby(postcode, coordinates) if source is not None else [postcode]
|
||||
|
||||
def _walk(
|
||||
self,
|
||||
postcodes: list[str],
|
||||
enough: Optional[Callable[[ComparableProperty], bool]],
|
||||
minimum: int,
|
||||
) -> tuple[list[ComparableProperty], int]:
|
||||
candidates: list[ComparableProperty] = []
|
||||
seen_certs: set[str] = set()
|
||||
matches = 0
|
||||
|
|
@ -136,7 +172,7 @@ class EpcComparablePropertiesRepository(ComparablePropertiesRepository):
|
|||
matches += 1
|
||||
if enough is not None and matches >= minimum:
|
||||
break
|
||||
return candidates
|
||||
return candidates, matches
|
||||
|
||||
# Mapper-shape errors: the cert's lodged data does not fit the schema/mapper.
|
||||
# `ValueError` — missing required field / unmapped code (`UnmappedApiCode`);
|
||||
|
|
|
|||
|
|
@ -245,6 +245,83 @@ def test_candidates_near_without_a_source_uses_only_the_seed() -> None:
|
|||
assert [c.certificate_number for c in candidates] == ["CERT-1"]
|
||||
|
||||
|
||||
def test_candidates_near_widens_once_when_the_normal_reach_is_insufficient() -> None:
|
||||
# Arrange — the normal-radius nearby set (P0, P1) has no MATCH cert; the
|
||||
# wider-radius set (P0, P1, P2, P3) reaches P3, which does.
|
||||
client = _MultiPostcodeEpcClient(
|
||||
{
|
||||
"P0": [_result("OTHER-1", uprn=1)],
|
||||
"P1": [_result("OTHER-2", uprn=2)],
|
||||
"P2": [_result("OTHER-3", uprn=3)],
|
||||
"P3": [_result("MATCH-1", uprn=4)],
|
||||
}
|
||||
)
|
||||
normal = _FakeNearbyPostcodes(["P0", "P1"])
|
||||
wider = _FakeNearbyPostcodes(["P0", "P1", "P2", "P3"])
|
||||
repo = EpcComparablePropertiesRepository(
|
||||
client,
|
||||
_FakeGeospatial({}),
|
||||
nearby_postcodes=normal,
|
||||
widen_nearby_postcodes=wider,
|
||||
)
|
||||
|
||||
# Act
|
||||
candidates = repo.candidates_near(
|
||||
"P0", None, enough=lambda c: c.certificate_number.startswith("MATCH"), minimum=1
|
||||
)
|
||||
|
||||
# Assert — both the normal and the widened source were consulted (in that
|
||||
# order), and the wider walk's matches are returned.
|
||||
assert normal.calls == [("P0", None)]
|
||||
assert wider.calls == [("P0", None)]
|
||||
certs = {c.certificate_number for c in candidates}
|
||||
assert "MATCH-1" in certs
|
||||
|
||||
|
||||
def test_candidates_near_does_not_widen_when_the_normal_reach_is_enough() -> None:
|
||||
# Arrange — the normal radius already yields the minimum match, so the
|
||||
# wider (more expensive) source must never be consulted.
|
||||
client = _MultiPostcodeEpcClient({"P0": [_result("MATCH-1", uprn=1)]})
|
||||
normal = _FakeNearbyPostcodes(["P0"])
|
||||
wider = _FakeNearbyPostcodes(["P0", "P1", "P2"])
|
||||
repo = EpcComparablePropertiesRepository(
|
||||
client,
|
||||
_FakeGeospatial({}),
|
||||
nearby_postcodes=normal,
|
||||
widen_nearby_postcodes=wider,
|
||||
)
|
||||
|
||||
# Act
|
||||
candidates = repo.candidates_near(
|
||||
"P0", None, enough=lambda c: c.certificate_number.startswith("MATCH"), minimum=1
|
||||
)
|
||||
|
||||
# Assert
|
||||
assert wider.calls == []
|
||||
assert [c.certificate_number for c in candidates] == ["MATCH-1"]
|
||||
|
||||
|
||||
def test_candidates_near_without_enough_predicate_never_widens() -> None:
|
||||
# Arrange — no `enough` predicate means the caller isn't gating on a match
|
||||
# count (e.g. the unconditional aggregate path), so widening is meaningless
|
||||
# and must not fire even with a configured wider source.
|
||||
client = _MultiPostcodeEpcClient({"P0": [_result("CERT-1", uprn=1)]})
|
||||
normal = _FakeNearbyPostcodes(["P0"])
|
||||
wider = _FakeNearbyPostcodes(["P0", "P1"])
|
||||
repo = EpcComparablePropertiesRepository(
|
||||
client,
|
||||
_FakeGeospatial({}),
|
||||
nearby_postcodes=normal,
|
||||
widen_nearby_postcodes=wider,
|
||||
)
|
||||
|
||||
# Act
|
||||
repo.candidates_near("P0", None)
|
||||
|
||||
# Assert
|
||||
assert wider.calls == []
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Unmappable cohort certs are skipped + recorded, not allowed to sink the cohort
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue