Fix corpsepose clipping error in interacted labor model (remove cache-order dependency)

[marinecdf]

Summary

This updates the corpsepose / plankpose clipping path in interpret/specification.py so transform(...) receives the clip-model reference curve directly, instead of reading it from othermodels[clipmodel].current_curves[region].

transform(...) now accepts two curves for corpsepose / plankpose:

1. the clip-model reference curve (minlost_lo)
2. the current model curve (minlost_hi)

and TransformCurveGenerator is wired to pass both curves into transform(...) when those clipping modes are used.

This is intended as a targeted temporary fix to remove the cache-order dependency in the case of corpsepose clipping (it was sufficient to complete both single and median interacted labor model projection runs in the current setup).

Why

The previous implementation depended on clip_curve = othermodels[specconf.get('clip-model')].current_curves[region] already being populated when transform(...) handled the corpsepose / plankpose clipping case.

That created an implicit cache-order dependency: if minlost_hi was evaluated before minlost_lo for a region, the cache entry (for the clip model i.e. minlost_lo) could be missing and the run would fail with error below.

This was the failure observed in the interacted labor projection (single run using this config file):

    [curvegen.get_curve(region, year, *args, **kw) for curvegen in self.curvegens]))
  File "/project/cil/home_dirs/mdefranciosi/repo/impact-calculations/interpret/specification.py", line 424, in transform
    clip_curve = othermodels[specconf.get('clip-model')].current_curves[region]
KeyError: 'CAN.1.2.28'

What Changed

• transform(...) in create_curvegen(...) now accepts one or more curves (*curves) instead of always a single curve.
• For corpsepose / plankpose, the transform now expects:
  • the clip-model reference curve
  • the current model curve
• The old cache lookup from current_curves[region] was removed from the corpsepose / plankpose clipping block.
• TransformCurveGenerator(...) is now passed both curve generators for corpsepose / plankpose, so the reference curve is supplied directly at transform time.

[jrising]

@marinecdf Can you also put the config you're using here?

[jrising]

I think the solution here is just to force minlost_lo to be evaluated before minlost_hi. That would be preferable to changing transform and TransformCurveGenerator, which are both fairly core functions in the projection system.

The nicest way to do this is to build a dependency tree and then execute it. That might be overkill, but it would make for a nice user interface.

[jrising]

There are a few reasons why we don't want this to be a permanent fix, but unfortunately I think it might be producing bad results already. You have to be very careful calling a curvegen multiple times for a given year, as I think this is doing. While I try to check for this elsewhere, there's a chance that any curve that models adaptation sees each year twice, and as a result updates its climate and economic covariates twice as fast as it should.

Have you tested your change with an existing model and made sure that it produces identical results? You could do that by creating a copy of minlost_hi in the config, and then clipping minlost_hi to minlost_hi_copy. That way, the clipping will be included but have no effect, and you can make sure that the numbers come out the same.

I will try to find time to implement a permanent fix here, but I don't know if I can do it this week.

[jrising]

Thinking about this more, I think your general approach is likely fine and we can turn it into a permanent fix with a couple changes. The extra calls to curvegen are totally appropriate. I just want you to check with a test like I describe above, because we have had problems with this in the past.