jixuan-chen / target

Python version of the TARGET model
2 stars 1 forks source link

include roofs error #11

Open biglimp opened 2 hours ago

biglimp commented 2 hours ago

I am still getting errors for a missing key:

Traceback (most recent call last):
File "C:\Users/xlinfr/AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\processing_umep\processor\target_algorithm.py", line 242, in processAlgorithm
tar.run_simulation(save_csv=False)
File "C:\OSGeo4W\apps\Python312\Lib\site-packages\target_py\scripts\toolkit.py", line 444, in run_simulation
ta_rslts = calc_ta(self.parameters, self.cfM, lc_data, grid, i, met_d, z_Uref, z_Hx2, Tb_rur, mod_data_ts_,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\OSGeo4W\apps\Python312\Lib\site-packages\target_py\scripts\Ta_module_new.py", line 124, in calc_ta
if cfM['include roofs'] == "Y":
~~~^^^^^^^^^^^^^^^^^
KeyError: 'include roofs'

Is this mandatory in config.ini and if so, what is the syntax since there is a space in the variable?

biglimp commented 2 hours ago

I fixed it but it looks like include roofs is mandatory, at least for me. Maybe this should be added into the test datasets...

jixuan-chen commented 1 hour ago

Yes, it is mandatory at the moment, but I wanted to discuss what parameters we'd like to make adjustable to the users. The Java version allows overriding a few parameters in the config file, including

Do we want to keep the Umep version consistent with the Java version in this regard? Or will it be challenging for setting up pre and post analysis tools (e.g. with changing output)? To me some parameters (e.g. reference heights) are important to include and some (e.g. include roofs) should have a default value or maybe just be excluded to not scare the users with too many input fields. Let me what you think and I can release a new PyPI version!