Open-Source-Spatial-Clean-Cooking-Tool / OnStove

This repository contain the general code for the Open Source Spatial Clean Cooking Tool OnStove
MIT License
7 stars 8 forks source link

Testing the package for first conda-forge release #345

Closed camiloramirezgo closed 1 year ago

camiloramirezgo commented 1 year ago
babakkhavari commented 1 year ago

For the first point: I think it is line 1770 in model.py of the main branch. If we remove that the zero should be gone. This should be fixed on my notebook branch

Fixed

babakkhavari commented 1 year ago

data.add_mask_layer(category='Administrative', name='Country_boundaries', path=adm_path)

Should not save the layer (save_layeris default = False), but it seems to save the layer anyway.

Ignore for now

babakkhavari commented 1 year ago

For add_layer we have the option of selecting normalization, but the only option for normalizationis MinMax in RasterLayer.normalization, should we change that to a boolean? It is fine like this, check docs are clear

Also, maybe I am doing something wrong, but I can not see any difference between inverse = True and inverse = False Check docs

It is also unclear what rescale does and what you should actually enter in the doc-string Check docs

Camilo / Fixed

babakkhavari commented 1 year ago

Could we stop this from happening?

Image

Test again, overwriting should be fine

Camilo / Worked fine

babakkhavari commented 1 year ago

warning when coordinate system has another unit than meters

When creating the DataProcessor warn that the CRS needs to be in meters and the resolution must be 1000x1000 meters. Update Documentation with the same

Babak

Fixed

babakkhavari commented 1 year ago

add_mask_layer only accepts vectors, which is fine, but maybe we can print a warning if the type is different than what is OK? Same thing for when ´base_layer´which only accepts rasters. Add warning Babak/FIXED

Same thing in the add_layer. For rasters maybe we can also give an option of resample? currently you can enter whatever you want, but I guess only certain options work? get list of methods from here and add warning Babak/FIXED

Still in these two functions can we give default values on category (maybe other?) and name (maybe the name of the input layer). These inputs may not completely clear? Add category default='Other' and Name=extractct name from path Camilo

Base layer is a little unclear. Here:

Image

it says that alignment ensures the resultion, crs and grids of all rasters match the what is entered in the start by the user. But we what is the base layer doing then? And is the baselayer also resampled? Improve description in NB and in docs Camilo / Fixed

In align_layers you can enter vectors, what happened when you do that? Warning to say that the layer is ignored Babak

babakkhavari commented 1 year ago

In read_scenario_data, can we also read files that have different separators than , might not be a problem in the training as we give them the data. But, different language settings are going to behave differently. Same thing when we read in the tech specs. This should be fine already, try testing it

Camilo / Tested works fine

babakkhavari commented 1 year ago

If someone runs the cell get_capactíty_cost without running the cell that reads in the model data in between it crashes. This is as far as I can see the only cell with this behavior. Worth doing something abt it?

Image

Explore why this happens

babakkhavari commented 1 year ago

In recalibrate_livestock we force people to include all livestock, would it make sense to let people use a selection?

This can be fixed, lets try

babakkhavari commented 1 year ago

Is this in percentage or ratio? i.e. is the max 100 or 1? I seem to be able to add whatever I want in these

Image

Improve description in NB and in docs

Camilo / Fixed

babakkhavari commented 1 year ago

population_to_dataframe creates the dataframe, while all other raster layers are extracted to it. So if you call a function that extracts raster data to the dataframe before the dataframe is creted (population is extracted) it crashes, can we instead print a warning?

Rename to create_dataframe and make explicit that the input layer must be the population

Babak

Skip for now

babakkhavari commented 1 year ago

Does it make sense to remove cells with less than 1 household?

We can remove all pop cells where hh < 1, before calibrating

babakkhavari commented 1 year ago

Image

Change this to emission costs avoided Yes

Babak Fixed