Skip to content

Refactor GCBM API #163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aornugent wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Refactor GCBM API #163

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8fa8826
@ankitaS11 Preprocessing refactor
aornugent Jul 31, 2022
322dd11
Update FLINTcloud reference config.
aornugent Jul 31, 2022
32fcfd0
Update: add tests using pytest, update GCBM API (more TODOs left)
ankitaS11 Aug 5, 2022
cecca2c
Merge branch 'master' into gcbm-api
ankitaS11 Aug 5, 2022
f25da75
Add input tiff files (Samples) for 2 simulations
ankitaS11 Aug 5, 2022
7c6a62d
Merge branch 'gcbm-api' of github.com:moja-global/FLINT.Cloud into gc…
ankitaS11 Aug 5, 2022
1436a61
Update local/rest_api_gcbm/gcbm.py
ankitaS11 Aug 5, 2022
eb62553
Support multiple attributes, generate provider_config, modules_cbm, a…
ankitaS11 Aug 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
210 changes: 139 additions & 71 deletions local/rest_api_gcbm/gcbm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,25 +33,33 @@ def is_category(self, path):
else:
return self.category in path

# Unlike list.append() in Python, this returns a bool - whether the append was successful or not + checks if the file path is of the current category
def _append(self, file_path):
if self.is_category(file_path):
def _append(self, file_path, check=True):
# Unlike list.append() in Python, this returns a bool - whether the append was successful or not + checks if the file path is of the current category
if check:
if self.is_category(file_path):
self.data.append(file_path)
return True
return False
else:
self.data.append(file_path)
return True
return False

def _update_config(self):
json_paths = {}
for file in self.data:
json_config_file = GCBMList.change_extension(file, ".json")
json_filepath = os.path.join(self.dirpath, json_config_file)

if json_config_file.name not in self.config:
self.generate_config(file, json_config_file)
else:
with open(json_filepath, "r+") as _config:
# TODO: This is not required I guess
with open(json_filepath, "w+") as _config:
json.dump(
self.files[file], _config, indent=4
)
json_paths[file] = json_filepath
return json_paths

def _populate_config_with_hard_coded_config(
self, config, hc_config, nodata
Expand All @@ -74,8 +82,8 @@ def generate_config(self, file, json_config_file):
json_filepath = os.path.join(self.dirpath, json_config_file)

mode = "w+"
if os.path.exists(json_filepath):
mode = "r+"
# if os.path.exists(json_filepath):
# mode = "w+"

# AO: disabling in favour of user defined attributes
# hard_coded_path = f"hard_coded_values/{json_config_file}"
Comment thread
aornugent marked this conversation as resolved.
Outdated
Expand All @@ -89,7 +97,10 @@ def generate_config(self, file, json_config_file):

with open(json_filepath, mode) as _file:
if mode == "r+":
config = json.load(_file)
try:
config = json.load(_file)
except:
breakpoint()
Comment thread
ankitaS11 marked this conversation as resolved.
Outdated
else:
config = dict()

Expand Down Expand Up @@ -134,7 +145,9 @@ def setattr(self, file, attributes):
config["has_year"] = True

self.files[file] = config
self._update_config()
# Output paths are returned to keep track
json_paths = self._update_config()
return json_paths

@staticmethod
def change_extension(file_path, new_extension):
Expand Down Expand Up @@ -174,7 +187,14 @@ class GCBMSimulation:
def __init__(self):
# create a global index
self.dirpath = "input/test-run"
self.config_dir_path = "templates"
self.files = {}

# Tracks the output path for the input received through `add_file` method
self.json_paths = {}

# TODO: Once categories are changed from strings to Enums, we should find a better way to have supported categories
self.supported_categories = ["classifiers", "disturbances", "miscellaneous"]
Copy link
Copy Markdown
Contributor Author

@aornugent aornugent Aug 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to support input databases also. If you rename /tests/tiffs/new_demo_run $\rightarrow$ tests/reference/new_demo_run then it makes sense to include files in addition to tiffs.

Copy link
Copy Markdown
Contributor Author

@aornugent aornugent Aug 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS. thank you for changing this to lowercase - CamelCase filenames drive me nuts.

Copy link
Copy Markdown
Contributor Author

@aornugent aornugent Aug 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P.P.S I also got sick of the unnecessary _moja suffix and removed it from the linux-demo reference simulation on #165

Copy link
Copy Markdown
Contributor

@ankitaS11 ankitaS11 Aug 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to support input databases also.

Got it! I understand that it's a TODO, and I'll add this feature in the next commit. Thanks for letting me know about the _moja suffix removal, since it's merged now - I'll rebase and fix these suffixes.

Copy link
Copy Markdown
Contributor

@ankitaS11 ankitaS11 Aug 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed! :)


# create sub-indices of different types
self.config = list()
Expand All @@ -188,16 +208,16 @@ def __init__(self):
self.miscellaneous = GCBMMiscellaneousList(files=self.files, config=self.config)

def create_simulation_folder(self):
if not os.path.exists(self.dirpath):
os.makedirs(self.dirpath)
for dir_path in [self.dirpath, self.config_dir_path]:
if not os.path.exists(dir_path):
os.makedirs(dir_path)


def create_file_index(self):
config_dir_path = "templates"
assert os.path.isdir(
config_dir_path
), f"Given config directory path: {config_dir_path} either does not exist or is not a directory."
for dirpath, _, filenames in os.walk(config_dir_path):
self.config_dir_path
), f"Given config directory path: {self.config_dir_path} either does not exist or is not a directory."
for dirpath, _, filenames in os.walk(self.config_dir_path):
for filename in filenames:
# Don't read any data, but create the json file
abs_filepath = os.path.abspath(os.path.join(dirpath, filename))
Expand All @@ -215,8 +235,9 @@ def create_file_index(self):
sim_filepath = os.path.join(self.dirpath, filename)
shutil.copy(abs_filepath, sim_filepath)


# file_path: disturbances (NOT MUST), classifiers (MUST), miscellaneous (MUST)
def add_file(self, file_path: str):
def add_file(self, file_path: str, category: str = ""):
"""
This function:

Expand All @@ -226,25 +247,50 @@ def add_file(self, file_path: str):
Parameters
==========
1. file_path (str), no default
2. category (str), default = "", if skipped - then the categories will be deduced from the file path
"""

# TODO: update to accept input from Flask endpoint
# FIXME: The flask end point should do the pre-processing to be able to only pass the `file_path` as a string
# TODO: update in app.py to send valid data to add_file
filename = os.path.basename(file_path)
shutil.copy(file_path, os.path.join(self.dirpath, filename))

if self.disturbances._append(filename):
self.disturbances._update_config()
return
if self.classifiers._append(filename):
self.classifiers._update_config()
return
if self.miscellaneous._append(filename):
self.miscellaneous._update_config()
return
# TODO: Add covariates here
def _disturbance(filename, check=True):
if self.disturbances._append(filename, check):
self.disturbances._update_config()

def _classifier(filename, check=True):
if self.classifiers._append(filename, check):
self.classifiers._update_config()

def _miscellaneous(filename, check=True):
if self.miscellaneous._append(filename, check):
self.miscellaneous._update_config()
Comment thread
aornugent marked this conversation as resolved.
Outdated

if category != "":
if category == "disturbances":
_disturbance(filename, check=False)
elif category == "classifiers":
_classifier(filename, check=False)
elif category == "miscellaneous":
_miscellaneous(filename, check=False)
else:
# We can also raise an error here
raise UserWarning(f"Given category {category} not supported, supported categories are: {disturbances, classifiers, miscellaneous}")
else:
print(f"Category wasn't provided, attempting to deduce it from the file path: {file_path}")
Copy link
Copy Markdown
Contributor Author

@aornugent aornugent Aug 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another scenario is that categories are provided as part of an HTTP request, via call to the /gcbm/upload REST endpoint.

@YashKandalkar - would you prefer to support individual file uploads on the front end, or the bulk upload option we currently use an an example (see: curl.md)

if self.disturbances._append(filename):
self.disturbances._update_config()
return
if self.classifiers._append(filename):
self.classifiers._update_config()
return
if self.miscellaneous._append(filename):
self.miscellaneous._update_config()
return
print(f"Couldn't deduce a valid from the file path {file_path}, supported categories: {self.supported_categories}")

# TODO
# self._save(file_path)

def sync_config(self, file_path):
def _write_to_file(file_path, data):
Expand All @@ -259,97 +305,119 @@ def _write_to_file(file_path, data):
# Also update the dict
self.files[file_path] = data


# TODO (@ankitaS11): We can just have these as class methods later, this will reduce the redundancy in the code later
def update_disturbance_config(self):
self.disturbances._update_config()
self.json_paths.update(self.disturbances._update_config())


def set_disturbance_attributes(self, file_path, payload):
self.json_paths.update(self.disturbances.setattr(file_path, payload))

def set_disturbance_attributes(self, file, payload):
self.disturbances.setattr(file, payload)

def update_classifier_config(self):
self.classifiers._update_config()
self.json_paths.update(self.classifiers._update_config())


def set_classifier_attributes(self, file_path, payload):
self.json_paths.update(self.classifiers.setattr(file_path, payload))

def set_classifier_attributes(self, file, payload):
self.classifiers.setattr(file, payload)

def update_miscellaneous_config(self):
self.miscellaneous._update_config()
self.json_paths.update(self.miscellaneous._update_config())


def set_miscellaneous_attributes(self, file_path, payload):
self.json_paths.update(self.miscellaneous.setattr(file_path, payload))


# TODO: category should be an enum instead of a string to avoid any mistakes
def set_attributes(self, category: str, file_path: str, payload: dict):
base_path = os.path.basename(file_path)
if category == "disturbances":
self.set_disturbance_attributes(base_path, payload)
elif category == "classifiers":
self.set_classifier_attributes(base_path, payload)
elif category == "miscellaneous":
self.set_miscellaneous_attributes(base_path, payload)
else:
raise UserWarning(f"Expected a valid category name out of {self.supported_categories}, but got {category}")

def set_miscellaneous_attributes(self, file, payload):
self.miscellaneous.setattr(file, payload)

@staticmethod
def safe_read_json(path):
def safe_read_json(file_path):

# TODO: add read method for gcbm_config.cfg and logging.conf
Copy link
Copy Markdown
Contributor

@ankitaS11 ankitaS11 Aug 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aornugent - Thanks for adding these TODOs in the code, they are extremely useful for me to track what needs to be done. However, I'm a bit confused about this one - do we want to just read these files, and store them in another folder? Or in a JSON? Or do we want to extract any information, and use it somewhere else? I'm not sure as I couldn't find a relevant part of it in app.py, so I'll really appreciate it if you can help me with this.

Copy link
Copy Markdown
Contributor Author

@aornugent aornugent Aug 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ankitaS11 - this one is a little cryptic. The two files need to be copied from templates and the default behaviour was to traverse all the config. files in the folder with safe_read_json, read their contents into the files dictionary, and then save a copy in the simulation folder.

But safe_read_json() only reads JSON, and was erroring out when given the .cfg and .conf files.

The contents of these files don't change very often (if ever!) but presently we have no way of reading them as part of the simulation configuration.

I'd appreciate your advice on how they can be handled.

Copy link
Copy Markdown
Contributor

@Crystalsage Crystalsage Aug 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @aornugent. I'm not Ankita, but I think configparser library would be a good choice for parsing the .conf files. They provide easy parsing for .ini style syntax.

For .cfg files, we can prepend a pseduo-section and then use configparser.

The code would look something like this:

import configparser

# For conf files
parser = configparser.ConfigParser()
parser.read('what.conf')
log = parser['Core']['DisableLogging'])

# For cfg files
parser = ConfigParser()
with open("gcbm_config.cfg") as config:
    parser.read_string("[top]\n" + config.read())

The problem is that all keys in the gcbm_config.cfg have the same name. This causes an error too.

config=localdomain.json
config=pools_cbm.json
config=modules_cbm.json
config=modules_output.json
config=spinup.json
config=variables.json
config=internal_variables.json

we can maybe rename some fields? Like localdomain_config=localdomain.json? :)

Copy link
Copy Markdown
Contributor Author

@aornugent aornugent Aug 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Crystalsage - that's a good suggestion. I don't think we can change the field names, because they're in the format that the GCBM understands.

Maybe we can just read these files in as strings for now?

Copy link
Copy Markdown
Contributor

@Crystalsage Crystalsage Aug 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aornugent Ah I see. In that case - what do you think about preparing a makeshift .cfg config in memory for better parsing and using that throughout the API? This way, we won't need to restructure the config file - and we can always change a few things around the API. A very rough sketch looks something like this:

import configparser

parser = configparser.ConfigParser()

gcbm_config_file = open("gcbm_config.cfg").read()
config_file_new = ""

# Build a makeshift config
for config in config_file.splitlines():
    # Get the json file name
    file_name = config.split('=')[1]

    # Trim off the extension to get the config type
    config_name = file_name[:-5]

    # Format into .ini style
    # e.g. localdomain_config=localdomain.json
    config_file_new += f"{config_name}_config={file_name}\n"

parser.read_string("[top]\n" + config_record)

# Returns 'localdomain.json'
parser['top']['localdomain_config']

Copy link
Copy Markdown
Contributor Author

@aornugent aornugent Sep 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Crystalsage - sorry for the delay. Your proposal looks great!

Copy link
Copy Markdown
Contributor Author

@aornugent aornugent Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Crystalsage - are you append your suggested changes to this branch please?

if ".cfg" in path:
if ".cfg" in file_path:
filename = os.path.join('input/test-run', "gcbm_config.cfg")
shutil.copy(path, filename)
shutil.copy(file_path, filename)
return {}
if ".conf" in path:

if ".conf" in file_path:
filename = os.path.join('input/test-run', "logging.conf")
shutil.copy(path, filename)
shutil.copy(file_path, filename)
return {}

# check JSON
if ".json" not in path:
raise UserWarning(f"Given path {path} not a valid json file")
return {}
if ".json" not in file_path:
raise UserWarning(f"Given path {file_path} not a valid json file")

# Make sure it's a file and not a directory
if not os.path.isfile(path):
if not os.path.isfile(file_path):
raise UserWarning(
f"Got a directory {path} inside the config directory path, skipping it."
f"Got a directory {file_path} inside the config directory path, skipping it."
)
return {}
with open(path, "r") as json_file:
with open(file_path, "r") as json_file:
data = json.load(json_file)
return data


if __name__ == "__main__":
sim = GCBMSimulation()
sim.add_file("tests/GCBM_New_Demo_Run/disturbances/disturbances_2011.tiff")
sim.set_disturbance_attributes("disturbances_2011.tiff", {"year": 2011, "disturbance_type": "Wildfire", "transition": 1})
sim.add_file("tests/tiff/new_demo_run/disturbances_2011_moja.tiff")
# sim.set_disturbance_attributes("disturbances_2011_moja.tiff", {"year": 2011, "disturbance_type": "Wildfire", "transition": 1})
sim.set_attributes(category="disturbances", file_path="disturbances_2011_moja.tiff", payload={"year": 2011, "disturbance_type": "Wildfire", "transition": 1})

sim.add_file("tests/GCBM_New_Demo_Run/disturbances/disturbances_2012.tiff")
sim.set_disturbance_attributes("disturbances_2012.tiff",
sim.add_file("tests/tiff/new_demo_run/disturbances_2012_moja.tiff")
sim.set_disturbance_attributes("disturbances_2012_moja.tiff",
{"year": 2012, "disturbance_type": "Wildfire", "transition": 1})


sim.add_file("tests/GCBM_New_Demo_Run/disturbances/disturbances_2013.tiff")
sim.set_disturbance_attributes("disturbances_2013.tiff",
sim.add_file("tests/tiff/new_demo_run/disturbances_2013_moja.tiff")
sim.set_disturbance_attributes("disturbances_2013_moja.tiff",
{"year": 2013, "disturbance_type": "Mountain pine beetle — Very severe impact", "transition": 1})

# TODO: Check how to handle multiple attributes entries (L442-451 of `app.py:master`)
# sim.set_disturbance_attributes("disturbances_2013.tiff",
# sim.set_disturbance_attributes("disturbances_2013_moja.tiff",
# {"year": 2013, "disturbance_type": "Wildfire", "transition": 1})

sim.add_file("tests/GCBM_New_Demo_Run/disturbances/disturbances_2014.tiff")
sim.set_disturbance_attributes("disturbances_2014.tiff",
sim.add_file("tests/tiff/new_demo_run/disturbances_2014_moja.tiff")
sim.set_disturbance_attributes("disturbances_2014_moja.tiff",
{"year": 2014, "disturbance_type": "Mountain pine beetle — Very severe impact", "transition": 1})

sim.add_file("tests/GCBM_New_Demo_Run/disturbances/disturbances_2015.tiff")
sim.set_disturbance_attributes("disturbances_2015.tiff",
sim.add_file("tests/tiff/new_demo_run/disturbances_2015_moja.tiff")
sim.set_disturbance_attributes("disturbances_2015_moja.tiff",
{"year": 2015, "disturbance_type": "Wildfire", "transition": 1})

sim.add_file("tests/GCBM_New_Demo_Run/disturbances/disturbances_2016.tiff")
sim.set_disturbance_attributes("disturbances_2016.tiff",
sim.add_file("tests/tiff/new_demo_run/disturbances_2016_moja.tiff")
sim.set_disturbance_attributes("disturbances_2016_moja.tiff",
{"year": 2016, "disturbance_type": "Wildfire", "transition": 1})

sim.add_file("tests/GCBM_New_Demo_Run/disturbances/disturbances_2018.tiff")
sim.set_disturbance_attributes("disturbances_2018.tiff",
sim.add_file("tests/tiff/new_demo_run/disturbances_2018_moja.tiff")
sim.set_disturbance_attributes("disturbances_2018_moja.tiff",
{"year": 2018, "disturbance_type": "Wildfire", "transition": 1})

# TODO: classifiers don't have 'year' attributes
sim.add_file("tests/GCBM_New_Demo_Run/classifiers/Classifier1.tiff")
# sim.set_classifier_attributes("classifier1.tiff",
sim.add_file("tests/tiff/new_demo_run/Classifier1_moja.tiff", category="classifiers")
# sim.set_classifier_attributes("Classifier1_moja.tiff",
Comment thread
aornugent marked this conversation as resolved.
Outdated
# {"1": "TA", "2": "BP", "3": "BS", "4": "JP", "5": "WS", "6": "WB", "7": "BF", "8": "GA"})

sim.add_file("tests/GCBM_New_Demo_Run/classifiers/Classifier2.tiff")
# sim.set_classifier_attributes("classifier1.tiff",
sim.add_file("tests/tiff/new_demo_run/Classifier2_moja.tiff", category="classifiers")
# sim.set_classifier_attributes("Classifier2_moja.tiff",
# {"1": "5", "2": "6", "3": "7", "4": "8"})

sim.add_file("tests/GCBM_New_Demo_Run/db/gcbm_input.db")
sim.add_file("tests/GCBM_New_Demo_Run/miscellaneous/initial_age.tiff")
sim.add_file("tests/GCBM_New_Demo_Run/miscellaneous/mean_annual_temperature.tiff")
sim.add_file("tests/tiff/new_demo_run/initial_age_moja.tiff", category="miscellaneous")
sim.add_file("tests/tiff/new_demo_run/mean_annual_temperature_moja.tiff", category="miscellaneous")

# TODO: make it work
# sim.add_file("tests/reference/gcbm_new_demo_run/gcbm_input.db")
Loading