IPESE / REHO

Decision Support Tool for Renewable Energy Communities
Apache License 2.0
6 stars 11 forks source link

[joss-review]: scripts/template/run.py fails #18

Closed hgandhi2411 closed 2 months ago

hgandhi2411 commented 6 months ago

Is there an existing issue for this?

What happened?

Used the pip installation as instructed in reho documentation. Two issues:

  1. To check proper installation, I still have to clone the repo
  2. /scripts/template/run.py fails to recognize the .env file (copied the example.env file and added ampl.lic path as well as my ampl key) placed in the root directory and fails to find another file Sion-hour.dat Attaching the complete error trace here.

Version

1.0.2 (Stable)

Relevant log output

AMPL_PATH is not defined. Please include a .env file at the project root (e.g., AMPL_PATH='C:/AMPL')
/opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/reho/model/preprocessing/QBuildings.py:426: UserWarning: Geometry column does not contain geometry.
  new_buildings_data[REHO_index] = df_buildings[key]
Traceback (most recent call last):
  File "/Users/hetagandhi/Documents/REHO/scripts/template/run.py", line 30, in <module>
    reho = reho(qbuildings_data=qbuildings_data, units=units, grids=grids, cluster=cluster, scenario=scenario, method=method, solver="highs")
  File "/opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/reho/model/reho.py", line 28, in __init__
    super().__init__(qbuildings_data, units, grids, parameters, set_indexed, cluster, method, solver, DW_params)
  File "/opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/reho/model/district_decomposition.py", line 65, in __init__
    self.File_ID = WD.get_cluster_file_ID(self.cluster)
  File "/opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/reho/model/preprocessing/weather.py", line 70, in get_cluster_file_ID
    df = read_hourly_dat(cluster['Location'])
  File "/opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/reho/model/preprocessing/weather.py", line 85, in read_hourly_dat
    df = np.loadtxt(os.path.join(path_to_weather, 'hour', location + '-hour.dat'), unpack=True, skiprows=1)
  File "/opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/numpy/lib/npyio.py", line 1373, in loadtxt
    arr = _read(fname, dtype=dtype, comment=comment, delimiter=delimiter,
  File "/opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/numpy/lib/npyio.py", line 992, in _read
    fh = np.lib._datasource.open(fname, 'rt', encoding=encoding)
  File "/opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/numpy/lib/_datasource.py", line 193, in open
    return ds.open(path, mode, encoding=encoding, newline=newline)
  File "/opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/numpy/lib/_datasource.py", line 533, in open
    raise FileNotFoundError(f"{path} not found.")
FileNotFoundError: /opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/reho/data/weather/hour/Sion-hour.dat not found.
(reho-review) hetagandhi@Abhisheks-Air template % ls ../../reho/data/weather/

Anything else?

No response

DorsanL commented 6 months ago
  1. To check proper installation, I still have to clone the repo

I clarified the « Checking proper installation » paragraph for an installation from PyPI or from source. Still, I recognize there is no automated test directly within the package. I guess we can discuss this in https://github.com/IPESE/REHO/issues/22 ?

  1. /scripts/template/run.py fails to recognize the .env file (copied the example.env file and added ampl.lic path as well as my ampl key) placed in the root directory

I updated the documentation and the paragraph AMPL license path, which redirects to REHO/Issues/Relative Path in VScode. I hope this will clarify things.

2b. and fails to find another file Sion-hour.dat

This file was indeed not available on the repository, my apologies for this. Fortunately, the use of local weather files is no longer necessary, since now REHO uses the pvlib library to connect to the PVGIS dabatase and to extract the weather data based on the building's coordinates. https://github.com/IPESE/REHO/commit/3ecf7cef15e0b50bf977cdc2f7fb9c34fe155b3d

Local weather files (xx-hour.dat) have therefore all been removed. https://github.com/IPESE/REHO/commit/47dec01c998555034f9e24452743eefb8ad6398c

hgandhi2411 commented 4 months ago

@DorsanL Thanks for removing redundant files.

I reinstalled reho using the pip install instructions. I still had the reho repo cloned. Tried to run run.py but still wasnt able to. A few suggestions based on the continued inability to run /scripts/template/run.py

  1. When running python /scripts/template/run.py from REHO root dir, I get this error:

    Traceback (most recent call last):
    File "/Users/hetagandhi/Documents/REHO/scripts/template/run.py", line 8, in <module>
    qbuildings_data = reader.read_csv(buildings_filename='data/buildings.csv', nb_buildings=1)
    File "/opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/reho/model/preprocessing/QBuildings.py", line 137, in read_csv
    self.data['buildings'] = file_reader(buildings_filename)
    File "/opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/reho/paths.py", line 74, in file_reader
    file = Path(path_handler(file))
    File "/opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/reho/paths.py", line 69, in path_handler
    raise FileNotFoundError('The relative path that was given is not a valid file.')
    FileNotFoundError: The relative path that was given is not a valid file.

    Please use relative paths in your paths.py file - these should be relative to the file being run rather than relative to the working directory.

  2. When I navigate to /scripts/template/ and run the file run.py file, the path issue is solved. However, there is a NameError:

    (reho-review) hetagandhi@Abhisheks-Air REHO % cd scripts/template 
    (reho-review) hetagandhi@Abhisheks-Air template % python run.py
    /opt/anaconda3/envs/reho-review/lib/python3.10/site-packages/reho/model/preprocessing/QBuildings.py:426: UserWarning: Geometry column does not contain geometry.
    new_buildings_data[REHO_index] = df_buildings[key]
    Traceback (most recent call last):
    File "/Users/hetagandhi/Documents/REHO/scripts/template/run.py", line 30, in <module>
    reho = REHO(qbuildings_data=qbuildings_data, units=units, grids=grids, cluster=cluster, scenario=scenario, method=method, solver="highs")
    NameError: name 'REHO' is not defined

    I believe this is because you haven't configured imports correctly in your __init__.py file. My recommendation is to configure your init files correctly.

  3. I tried starting a python shell to test reho install. Here's the output - confirming my suspicion of imports not configured properly in your init.py files

    image
  4. Please also set up some basic tests to test import issues, and fundamental functions so that review of this package can be accelerated. As stated in #22.

DorsanL commented 3 months ago

Thank you for the feedback.

Currently, most paths in the code are defined with os.path rather than Path. Unfortunately, we had noticed that os.get_cwd() command leads to different results when running the script from Pycharm (path of the file itself) or from VScode or terminal (path of the working directory). Cf corresponding issue here.

However, the Path(__file__).parent function seems to always give the same result (whether running from Pycharm / VScode / terminal), and appears to solve the problem of relative paths when loading files such as working_directory/data/filename.csv.

Do you recommend using Path objects everywhere (code would need a bit of adaptation)? More specifically, what would you recommend instead of defining the relative paths based on os.path.dirname(__file__) in paths.py ?

  1. scripts/test/run.py should now work.
  2. and 3. I added from . import reho in model/__init__.py, does it solve your problem?
  3. I added some entry points for testing the package without having to download the full (or parts of the) repository. I also included the pytest package to verify proper imports (https://github.com/IPESE/REHO/commit/84feda1aa251d7a3879843b6280b399b06cb996b). Any further suggestion is welcome :)