Interpolate between two scenarios

[aaccensi]

Description

Adds the capability to interpolate between two scenarios with the API endpoint: POST /api/v3/scenarios/interpolate
Now if we pass a start_scenario_id besides the mandatory end_year it will interpolate using the values of the given start scenario instead of the starting values of the scenario targeted by the endpoint.

Type of change

• Bug fix
• New feature
• Enhancement
• Documentation

Checklist

• I have tested these changes
• I have updated documentation as needed (Work in progress)
• I have tagged the relevant people for review

Related Issues

Closes #1684

[noracato]

Nice work! Especially on the specs! Thank you for adding the extra spec for the API as well.

It works well as an extension of the existing code. I do think we can win some space by replacing the now many validators by introducing simple Dry validation rules. @louispt1 has been working with them a lot lately, he should be able to help you out.

Again well done!

[louispt1]

Apart from the one remark below and the spec for the batch endpoint I'd say it's looking good!

[noracato]

The implementation of Dry and the rulesets went well. Thanks for trying it out!

There are a few optimizations to be made regarding Rails. You can check my comments in the code. If you need any more explanations or resources, please ask!

[noracato]

Let's move this to the top of the file with the rest of the authorize statements. Like this they're all together and we keep the overview.

[noracato]

When instantiating a memoized var, it is a good practice to do this in a separate method. Like this, when changing the order of fetch_and_validate_scenarios and validate_target_years being called will never be a problem.

Also, .to_a will force the relation object onto an array. You will loose a lot of optimizations by Rails here. The relation object can do most of what an array can do, and will suffice here.

def scenarios
  @scenarios ||= Scenario.accessible_by(@ability).where(id: @scenario_ids).sort_by(&:end_year)
end


def validate_scenarios
  if scenarios.length != @scenario_ids.length
    return Failure(scenario_ids: ["scenarios not found: #{(@scenario_ids - scenarios.map(&:id)).join(', ')}"])
  end
end

[noracato]

It might be worth it to functionally separate a validation class and an interpolate class, like the rulesets you created in the YearInterpolator. But it's also fine to leave it like this for now.

[noracato]

Suggested change

	@scenarios.each do |scenario|
	if scenario.scaler
	return Failure(scenario_ids: ["cannot interpolate scaled scenario #{scenario.id}"])
	end
	if scenario.start_year != first.start_year
	return Failure(scenario_ids: ['all scenarios must have the same start year'])
	end
	if scenario.area_code != first.area_code
	return Failure(scenario_ids: ['all scenarios must have the same area code'])
	end
	end
	if scenarios.any?(&:scaler)
	return Failure(scenario_ids: ["cannot interpolate scaled scenarios"])
	end
	unless scenarios.uniq(&:area_code).length == 1
	return Failure(scenario_ids: ['all scenarios must have the same area code'])
	end

Area code and start year are linked. So if one matches, the other matches as well. Shortly: each area_code can have only one start year. Meaning we only need to check the area code here.

We can also use some optimized Rails methods for looping. Like any and unique.

[noracato]

Suggested change

	results = []
	@end_years.each do |target_year|
	results = @end_years.reduce do |target_year|

[noracato]

Suggested change

	case result
	in Dry::Monads::Success(scenario)
	results << scenario
	in Dry::Monads::Failure(errors)
	msg = "failed to interpolate year #{target_year}: #{errors.values.flatten.join(', ')}"
	return Failure(interpolation: [msg])
	end
	if result.success?
	result.value!
	else
	msg = "failed to interpolate year #{target_year}: #{result.errors.values.flatten.join(', ')}"
	return Failure(interpolation: [msg])
	end

[noracato]

Suggested change

	earlier_scenario&.id,
	earlier_scenario,

Why pass only the id here and then retrieve the scenario from the db again in the YearInterpolator.