From bd71dee7925c39e9ba9d89bf9ea797b92fce82bd Mon Sep 17 00:00:00 2001 From: andrea rota Date: Tue, 23 Apr 2024 12:53:42 +0100 Subject: [PATCH 1/3] filter out feature amounts per PU rows where amount is not a positive number [MRXNM-43] Reason: the Marxan solver will show unexpected behaviour when it sees `puvspr.dat` rows with amount = 0. --- ...memory-feature-amounts-per-planning-unit.repository.ts | 8 ++++++-- ...ypeorm-feature-amounts-per-planning-unit.repository.ts | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/api/libs/feature-amounts-per-planning-unit/src/repository/memory-feature-amounts-per-planning-unit.repository.ts b/api/libs/feature-amounts-per-planning-unit/src/repository/memory-feature-amounts-per-planning-unit.repository.ts index a5a30f6faa..24ab06806c 100644 --- a/api/libs/feature-amounts-per-planning-unit/src/repository/memory-feature-amounts-per-planning-unit.repository.ts +++ b/api/libs/feature-amounts-per-planning-unit/src/repository/memory-feature-amounts-per-planning-unit.repository.ts @@ -26,8 +26,12 @@ export class MemoryFeatureAmountsPerPlanningUnitRepository if (!featureAmountsPerPlanningUnit) return []; - return featureAmountsPerPlanningUnit.filter(({ featureId }) => - featureIds.includes(featureId), + return ( + featureAmountsPerPlanningUnit + .filter(({ featureId }) => featureIds.includes(featureId)) + /** The Marxan solver will show unexpected behaviour when seeing + * puvspr.dat rows with amount = 0 */ + .filter(({ amount }) => amount > 0) ); } async saveAmountPerPlanningUnitAndFeature( diff --git a/api/libs/feature-amounts-per-planning-unit/src/repository/typeorm-feature-amounts-per-planning-unit.repository.ts b/api/libs/feature-amounts-per-planning-unit/src/repository/typeorm-feature-amounts-per-planning-unit.repository.ts index 5fe71e9284..253f906855 100644 --- a/api/libs/feature-amounts-per-planning-unit/src/repository/typeorm-feature-amounts-per-planning-unit.repository.ts +++ b/api/libs/feature-amounts-per-planning-unit/src/repository/typeorm-feature-amounts-per-planning-unit.repository.ts @@ -40,6 +40,9 @@ export class TypeormFeatureAmountsPerPlanningUnitRepository .from(FeatureAmountsPerPlanningUnitEntity, 'fappu') .where('project_id = :projectId', { projectId }) .andWhere('feature_id IN (:...featureIds)', { featureIds }) + /** The Marxan solver will show unexpected behaviour when seeing + * puvspr.dat rows with amount = 0 */ + .andWhere('amount > 0') .execute(); } From d6ce175c4e8eef8a17fa10101b1af4e3ba55d40c Mon Sep 17 00:00:00 2001 From: andrea rota Date: Tue, 23 Apr 2024 12:55:42 +0100 Subject: [PATCH 2/3] apply source formatting --- ...re-amounts-per-planning-unit.repository.ts | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/api/libs/feature-amounts-per-planning-unit/src/repository/typeorm-feature-amounts-per-planning-unit.repository.ts b/api/libs/feature-amounts-per-planning-unit/src/repository/typeorm-feature-amounts-per-planning-unit.repository.ts index 253f906855..afc942b97a 100644 --- a/api/libs/feature-amounts-per-planning-unit/src/repository/typeorm-feature-amounts-per-planning-unit.repository.ts +++ b/api/libs/feature-amounts-per-planning-unit/src/repository/typeorm-feature-amounts-per-planning-unit.repository.ts @@ -32,18 +32,20 @@ export class TypeormFeatureAmountsPerPlanningUnitRepository projectId: string, featureIds: string[], ): Promise { - return this.geoEntityManager - .createQueryBuilder() - .select('amount') - .addSelect('project_pu_id', 'projectPuId') - .addSelect('feature_id', 'featureId') - .from(FeatureAmountsPerPlanningUnitEntity, 'fappu') - .where('project_id = :projectId', { projectId }) - .andWhere('feature_id IN (:...featureIds)', { featureIds }) - /** The Marxan solver will show unexpected behaviour when seeing - * puvspr.dat rows with amount = 0 */ - .andWhere('amount > 0') - .execute(); + return ( + this.geoEntityManager + .createQueryBuilder() + .select('amount') + .addSelect('project_pu_id', 'projectPuId') + .addSelect('feature_id', 'featureId') + .from(FeatureAmountsPerPlanningUnitEntity, 'fappu') + .where('project_id = :projectId', { projectId }) + .andWhere('feature_id IN (:...featureIds)', { featureIds }) + /** The Marxan solver will show unexpected behaviour when seeing + * puvspr.dat rows with amount = 0 */ + .andWhere('amount > 0') + .execute() + ); } public async saveAmountPerPlanningUnitAndFeature( From 9c5c8ddcf4e881ca119086d344fba98d752195a0 Mon Sep 17 00:00:00 2001 From: andrea rota Date: Tue, 23 Apr 2024 13:03:22 +0100 Subject: [PATCH 3/3] add test: no rows with amount=0 should be generated for puvspr.dat files --- .../compute-area.service.spec.ts | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/api/apps/api/src/modules/scenarios-features/compute-area.service.spec.ts b/api/apps/api/src/modules/scenarios-features/compute-area.service.spec.ts index 1a816ec9ed..b65b8cc474 100644 --- a/api/apps/api/src/modules/scenarios-features/compute-area.service.spec.ts +++ b/api/apps/api/src/modules/scenarios-features/compute-area.service.spec.ts @@ -30,6 +30,21 @@ describe(ComputeArea, () => { await fixtures.ThenMinMaxAmountWasSaved(); }); + it('does not include rows with amount = 0 in puvspr.dat files', async () => { + const { projectId, scenarioId } = fixtures.GivenProject(); + const featureId = + fixtures.GivenFeatureWasCreatedWithSomeAmountsPerPuEqualToZero(); + fixtures.GivenMinMaxAmount(featureId, undefined, undefined); + + await fixtures.WhenComputing(projectId, scenarioId, featureId); + + await fixtures.ThenNoPuvsprRowsWithAmountEqualToZeroShouldBeGenerated( + projectId, + featureId, + ); + await fixtures.ThenMinMaxAmountWasSaved(); + }); + it('does not save saves amounts per planning unit when computation has already been made but saves min/max amount for the feature', async () => { const { projectId, scenarioId } = fixtures.GivenProject(); const featureId = fixtures.GivenNoComputationHasBeenSaved(); @@ -111,6 +126,7 @@ const getFixtures = async () => { sandbox.get(FeatureAmountsPerPlanningUnitRepository); const expectedPuid = v4(); + const expectedPuidWithAmountEqualToZero = v4(); const expectedAmount = 20; return { GivenProject: () => { @@ -146,6 +162,27 @@ const getFixtures = async () => { return featureId; }, + GivenFeatureWasCreatedWithSomeAmountsPerPuEqualToZero: () => { + const featureId = v4(); + computeMarxanAmountPerPlanningUnitMock.mockImplementation(async () => { + return [ + { + featureId, + projectPuId: expectedPuid, + amount: expectedAmount, + puId: 1, + }, + { + featureId, + projectPuId: expectedPuidWithAmountEqualToZero, + amount: 0, + puId: 2, + }, + ]; + }); + + return featureId; + }, GivenComputationAlreadySaved: (projectId: string, featureId: string) => { featureAmountsPerPlanningUnitRepo.memory[projectId] = [ { featureId, amount: 42, projectPuId: v4() }, @@ -177,6 +214,29 @@ const getFixtures = async () => { featureId, }); }, + ThenNoPuvsprRowsWithAmountEqualToZeroShouldBeGenerated: async ( + projectId: string, + featureId: string, + ) => { + const savedCalculations = + await featureAmountsPerPlanningUnitRepo.getAmountPerPlanningUnitAndFeature( + projectId, + [featureId], + ); + + expect(savedCalculations).toBeDefined(); + expect(savedCalculations[0]).toEqual({ + amount: expectedAmount, + projectPuId: expectedPuid, + featureId, + }); + expect(savedCalculations).not.toContain({ + amount: 0, + projectPuId: expectedPuidWithAmountEqualToZero, + featureId, + }); + expect(savedCalculations.length).toBe(1); + }, ThenComputationsWasNotDone: async () => { expect(computeMarxanAmountPerPlanningUnitMock).not.toHaveBeenCalled(); },