[feature] Refactor crop models outputs - homogenize

Summary

Outputs of crop models should be identical to homogenize analysis/visualization methods.

Current behavior

run_crop_simu outputs two objects : Soil_plot and Crop_plot. This is based on Simple formalism. Problems :

• Stics outputs only one object (crop). Therefore, Soil_plot is left empty. This is not a blocking issue because Soil plot can simply be ignored
• Grassim outputs three objects (crop, soil, management). For, now, crop and management outputs are stored in crop_plot. Analaysis/visualisation needs to be run with crop_plot[0/1] instead of Crop_plot directly.

Desired behavior

run_crop_simu should return a single "result" or "output" object, that visualization functions could take as an input. 2 options :

• concatenate all 'nyears_data' dictionaries in one
• Le the analysis/visualization functions iterate over the "result" object to find the right dictionary

Proposition 1 : run_crop_simu returns a dictionary 'results', containing the output objects as values and names as keys. Ex :

def run_crop_simu(...):
  results = {}
  ...
  #Simple 
  results['Soil'], results['Crop'] = run_independant_years_of_crop()
  # Stics
  results['Crop'] = run_independant_usms() 
  # Grassim
  results['Soil'], results['Crop'], results['Management'] = run_grassim(...)

  return results

Proposition 2 : run_crop_simu returns a dictionary 'results', concatenation of all nyears_data dictionaries.

def run_crop_simu(...):
  results = {}
  ...
  #Simple 
  Soil, Crop = run_independant_years_of_crop()
  results = Soil.nyears_data
  results.update(Crop.nyears_data)
  
  # Stics
  Crop = run_independant_usms()
  results = Crop.nyears_data 
  
  # Grassim
  Soil, Crop, Management = run_grassim(...)
  results = Soil.nyears_data
  results.update(Crop.nyears_data)
  results.update(Management.nyears_data)

  return results

Sources

(Give relevant sources i.e. stackoverflow posts, scientific papers, etc.)

Linked features or branches

(List relevant linked features and branches of the project, typically if this feature request is the child of another branch)

added Priority 3 - Week Severity 1 - blocker crop needs specifics outputs statusto do labels

set weight to 2

assigned to @Roxane.Bruhwyler and @Louis.Lemaire

changed the description

changed the description

changed the description

@abouvry @Roxane.Bruhwyler What do you think about these two implementation propositions ?

I personally prefer the second options.

pros :

• no need to iterate over a dictionary,
• more straightforward structure,
• no useless temporary variables stored in the output object
• no confusion between the output object ('result' dictionary made only of years_data) and the runtime objects (Crop, Soil, Management).

cons :

• Must be careful not to have conflicting key names between nyears_data dictionaries,
• Information about the runtime object (attributes, methods) is not accessible in the output object.

Pro/cons of the 1st option is basically the opposite.

Both methods are agnostic of the number of objects used at runtime.

Option 2 works for me !

Option 2 also ok for me.

created branch 126-feature-refactor-crop-models-outputs-homogenize to address this issue

mentioned in merge request !34 (merged)

Changes are pushed, run_crop_simu() returns results, a concatenation of nyears_data_dict. Of course visualize_map_of_a_variable() doesn't work anymore, @Roxane.Bruhwyler could you change it accordingly ? This function should probably be in MODULES.DATA_MANAGEMENT.output and take results dictionary as an argument, instead of being copied as a method of each runtime model class.

removed needs specifics label

mentioned in issue #128 (closed)

mentioned in merge request !36 (merged)

mentioned in commit 794124e4

mentioned in commit f86e9b4d

closed