architecture-building-systems / CityEnergyAnalyst

The City Energy Analyst (CEA)
https://www.cityenergyanalyst.com/
MIT License
196 stars 66 forks source link

Replace whitespaces with underscore in building name #3361

Closed reyery closed 1 year ago

reyery commented 1 year ago

This should fix an issue where network simulations would fail when the are whitespaces in building names.

ShiZhongming commented 1 year ago

@reyery @MatNif

Before this PR, were there any scenarios with building names containing space that failed in the new optimisation script?

I have just tested the master script with the new optimisation. It ran through successfully using both single-processing and multi-processing.

However, when testing this PR using the same scenario: single-processing ran through while multi-processing did not. The error is as follows.

`The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/zshi/Documents/GitHub/CityEnergyAnalyst/cea/worker.py", line 146, in worker
    run_job(config, job, server)
  File "/Users/zshi/Documents/GitHub/CityEnergyAnalyst/cea/worker.py", line 108, in run_job
    script(config=config, **parameters)
  File "/Users/zshi/Documents/GitHub/CityEnergyAnalyst/cea/api.py", line 60, in __call__
    self._runner.__call__(*args, **kwargs)
  File "/Users/zshi/Documents/GitHub/CityEnergyAnalyst/cea/api.py", line 38, in script_runner
    script_module.main(config)
  File "/Users/zshi/Documents/GitHub/CityEnergyAnalyst/cea/optimization_new/domain.py", line 452, in main
    current_domain.optimize_domain()
  File "/Users/zshi/Documents/GitHub/CityEnergyAnalyst/cea/optimization_new/domain.py", line 180, in optimize_domain
    non_dominated_fronts = toolbox.map(toolbox.evaluate, population)
  File "/Users/zshi/micromamba/envs/cea/lib/python3.8/multiprocessing/pool.py", line 364, in map
    return self._map_async(func, iterable, mapstar, chunksize).get()
  File "/Users/zshi/micromamba/envs/cea/lib/python3.8/multiprocessing/pool.py", line 771, in get
    raise self._value
wntr.epanet.toolkit.EpanetException: EPANET Error 200`
MatNif commented 1 year ago

@shizhongming, thanks for testing this.

@reyery ran into this exact same EPANET error when he ran the script with building names that contained whitespace. I tried to recreate the same error yesterday and did not manage to do so either.

I'm currently in a bit of a rush to finish a report for CS, but if you can both send me your test cases I'll take a closer look at this error in a few weeks.

ShiZhongming commented 1 year ago

@MatNif Take your time and I just sent you via Slack. See you tomorrow.

reyery commented 1 year ago

I can just think of one case, where this fix might create an issue further down the line and that is if the building name already contains an underscore.

@MatNif actually I'm not sure how this would affect buildings that already have an underscore, is it when it parses the results of wntr?

MatNif commented 1 year ago

I can just think of one case, where this fix might create an issue further down the line and that is if the building name already contains an underscore.

@MatNif actually I'm not sure how this would affect buildings that already have an underscore, is it when it parses the results of wntr?

Sorry, you are right, my bad. For some reason I thought line 496 was reversing line 486, but that is clearly not the case. So, I think you can safely ignore my earlier comment.