Skip to content
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.

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

Feature/fix bm #146

Merged
merged 8 commits into from
Feb 18, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
various
fix test failures, crash with NASA data, wrong path assignment, update tutorial
  • Loading branch information
aleeciu committed Feb 15, 2021
commit acdc729253230119830b5d7ec0f3c9e372f036ac
2 changes: 1 addition & 1 deletion climada/entity/exposures/black_marble.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def set_countries(self, countries, ref_year=2016, res_km=None, from_hr=None,
independently of its year of acquisition.
admin_file (str): file name, admin_0_countries or admin_0_map_subunits
kwargs (optional): 'gdp' and 'inc_grp' dictionaries with keys the
country ISO_alpha3 code. 'poly_val' polynomial transformation
country ISO_alpha3 code. 'poly_val' list of polynomial coefficients
[1,x,x^2,...] to apply to nightlight (DEF_POLY_VAL used if not
provided). If provided, these are used.
"""
Expand Down
26 changes: 14 additions & 12 deletions climada/entity/exposures/nightlight.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@
import numpy as np
import scipy.sparse as sparse
import matplotlib.pyplot as plt
from PIL import Image

from .litpop import read_bm_file
from .litpop import read_bm_file, get_bm
from climada.util import ureg
from climada.util.constants import SYSTEM_DIR
from climada.util.files_handler import download_file
from climada.util.save import save
from PIL import Image
tovogt marked this conversation as resolved.
Show resolved Hide resolved

Image.MAX_IMAGE_PIXELS = 1e9

Expand Down Expand Up @@ -252,8 +252,10 @@ def load_nightlight_nasa(bounds, req_files, year):
if np.any(extent[1, :] < 0) or np.any(extent[0, :] >= NASA_TILE_SIZE):
# this tile does not intersect the specified bounds
continue
extent = np.int64(np.clip(extent, 0, tile_size[None] - 1))
extent = np.int64(np.clip(extent, 0, tile_size[None] - 1))

tovogt marked this conversation as resolved.
Show resolved Hide resolved
im_nl, _ = read_bm_file(SYSTEM_DIR, fname.replace('*', str(year)))
im_nl = np.flipud(im_nl)
im_nl = sparse.csc.csc_matrix(im_nl)
im_nl = im_nl[extent[0, 0]:extent[1, 0] + 1, extent[0, 1]:extent[1, 1] + 1]
nightlight.append((tile_coord, im_nl))
Expand All @@ -280,17 +282,17 @@ def unzip_tif_to_py(file_gz):
sparse.csr_matrix (nightlight)
"""
LOGGER.info("Unzipping file %s.", file_gz)
file_path = Path(Path(file_gz).stem)
file_name = Path(Path(file_gz).stem)
with gzip.open(file_gz, 'rb') as f_in:
with file_path.open('wb') as f_out:
with file_name.open('wb') as f_out:
shutil.copyfileobj(f_in, f_out)
nightlight = sparse.csc.csc_matrix(plt.imread(file_path))
nightlight = sparse.csc.csc_matrix(plt.imread(file_name))
# flip X axis
nightlight.indices = -nightlight.indices + nightlight.shape[0] - 1
nightlight = nightlight.tocsr()
file_path.unlink()
file_name = file_path.stem + ".p"
save(file_name, nightlight)
file_name.unlink()
file_path = SYSTEM_DIR.joinpath(file_name.stem + ".p")
save(file_path, nightlight)

return file_name, nightlight

Expand Down Expand Up @@ -362,7 +364,7 @@ def load_nightlight_noaa(ref_year=2013, sat_name=None):
for pre_i in np.arange(ini_pre, end_pre, -1):
url = NOAA_SITE + 'F' + str(pre_i) + str(ref_year) + '.v4.tar'
try:
file_down = download_file(url)#, download_dir=SYSTEM_DIR)
file_down = download_file(url, download_dir=SYSTEM_DIR)
Copy link
Collaborator

Choose a reason for hiding this comment

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

i'm still not fully acquainted with the outcomes of the new config settings, but shouldn't file downloads etc. be configurable by setting the path in the config accordingly, as opposed to hard-coding system_dir as the option of choice here?

Copy link
Collaborator Author

@aleeciu aleeciu Feb 15, 2021

Choose a reason for hiding this comment

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

That's what I thought too.. but it was not the case for me when I ran it, it was saving the downloaded files in my working directory

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe @emanuel-schmid can comment on this?

I'm not sure what exactly is happening here, but the rest of this PR is fine from my side.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yep, same from my side, all good, but would be nice if Emanuel could explain / help out on this one.

Copy link
Collaborator

@emanuel-schmid emanuel-schmid Feb 17, 2021

Choose a reason for hiding this comment

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

The parameter download_dir of the download_file functions was introduced with the changes of the config settings.
(Before the changes, it used to download files into the working directory. Which was sometimes countered by undesired os.chdir() calls.)
The function's default directory is CONFIG.local_data.save_dir. The configuration's default value for this is ./results.

SYSTEM_DIR is a constant, but it's not exactly hard coded. Its value is defined as CONFIG.local_data.system.

I hope these explanations help clarify the behaviour?

Copy link
Collaborator

Choose a reason for hiding this comment

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

SYSTEM_DIR is a constant, but it's not exactly hard coded. Its value is defined as CONFIG.local_data.system.

Thanks! I think that is the answer to @Evelyn-M's original question and solves this confusion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks Emanuel, yes, it was creating a ./results folder in the working directory and saving files therein. Problem is, the loading function called after was expecting the files to be in SYSTEM_DIR, so that was the problem.

So maybe that also explaines why the same did not crash on Jenkins? (see #143) Can it be that there it is CONFIG.local_data.save_dir == CONFIG.local_data.system ? (my guess)

break
except ValueError:
pass
Expand All @@ -373,7 +375,7 @@ def load_nightlight_noaa(ref_year=2013, sat_name=None):
else:
url = NOAA_SITE + sat_name + str(ref_year) + '.v4.tar'
try:
file_down = download_file(url)#, download_dir=SYSTEM_DIR)
file_down = download_file(url, download_dir=SYSTEM_DIR)
except ValueError:
LOGGER.error('Nightlight intensities for year %s and satellite'
' %s do not exist.', ref_year, sat_name)
Expand All @@ -386,4 +388,4 @@ def load_nightlight_noaa(ref_year=2013, sat_name=None):
coord_nl[0, :] = [NOAA_BORDER[1], NOAA_RESOLUTION_DEG]
coord_nl[1, :] = [NOAA_BORDER[0], NOAA_RESOLUTION_DEG]

return nightlight, coord_nl, fn_light
return nightlight, coord_nl, fn_light
13 changes: 6 additions & 7 deletions climada/test/test_blackmarble.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def test_sint_maarten_pass(self):
ent = BlackMarble()
with self.assertLogs('climada.util.finance', level='INFO') as cm:
ent.set_countries(country_name, 2013, res_km=0.2)
self.assertIn('GDP SXM 2014: 3.658e+08.', cm.output[0])
self.assertIn('GDP SXM 2013: 1.023e+09.', cm.output[0])
self.assertIn('Income group SXM 2013: 4.', cm.output[1])
self.assertTrue(equal_crs(ent.crs['init'], {'init': 'epsg:4326'}))

Expand All @@ -74,7 +74,7 @@ def test_sint_maarten_pass(self):
cm.output[0])
self.assertIn("Processing country Sint Maarten.", cm.output[1])
self.assertIn("Generating resolution of approx 0.2 km.", cm.output[2])
self.assertAlmostEqual(ent.value.sum(), 3.658e+08 * (4 + 1))
self.assertTrue(np.isclose(ent.value.sum(), 1.023e+09 * (4 + 1), 1))
self.assertTrue(equal_crs(ent.crs['init'], {'init': 'epsg:4326'}))

def test_anguilla_pass(self):
Expand Down Expand Up @@ -131,7 +131,6 @@ def test_from_hr_flag_pass(self):
print('MemoryError caught')
pass


ent = BlackMarble()
with self.assertLogs('climada.entity.exposures.black_marble', level='INFO') as cm:
ent.set_countries(country_name, 2012, res_km=5.0, from_hr=False)
Expand All @@ -146,8 +145,8 @@ class BMFuncs(unittest.TestCase):
def test_cut_nasa_esp_pass(self):
"""Test load_nightlight_nasa function."""
shp_fn = shapereader.natural_earth(resolution='10m',
category='cultural',
name='admin_0_countries')
category='cultural',
name='admin_0_countries')
tovogt marked this conversation as resolved.
Show resolved Hide resolved
shp_file = shapereader.Reader(shp_fn)
list_records = list(shp_file.records())
for info_idx, info in enumerate(list_records):
Expand Down Expand Up @@ -176,9 +175,9 @@ def test_load_noaa_pass(self):
self.assertEqual(coord_nl[0, 0], NOAA_BORDER[1])
self.assertEqual(coord_nl[1, 0], NOAA_BORDER[0])
self.assertEqual(coord_nl[0, 0] + (nightlight.shape[0] - 1) * coord_nl[0, 1],
NOAA_BORDER[3])
NOAA_BORDER[3])
tovogt marked this conversation as resolved.
Show resolved Hide resolved
self.assertEqual(coord_nl[1, 0] + (nightlight.shape[1] - 1) * coord_nl[1, 1],
NOAA_BORDER[2])
NOAA_BORDER[2])
tovogt marked this conversation as resolved.
Show resolved Hide resolved

def test_set_country_pass(self):
"""Test exposures attributes after black marble."""
Expand Down
Loading