cire-thk / bifacialsimu Goto Github PK
View Code? Open in Web Editor NEWSimulation of bifacial PV
License: BSD 3-Clause "New" or "Revised" License
Simulation of bifacial PV
License: BSD 3-Clause "New" or "Revised" License
The repository currently has a BifacialSimu.egg-info
directory. I suggest deleting it (and potentially adding it to the .gitignore
) since it is an artifact of the build and not actually part of the repository's source code.
Cross-reference: openjournals/joss-reviews#4443
FileNotFoundError: [Errno 2] No such file or directory: 'C:\Users\lizhen\anaconda3\envs\bifacialsimu\Lib\Outputs\2022 11 30_17_24_results//radiation_qabs_results.csv'
This error occurs when running both modes 3 and 5.
I see that the latest version on PyPI is 1.0.2
, but setup.py
still lists 1.0.0
:
Line 29 in 158ca37
Something to consider: rather than manually updating a hardcoded version string in setup.py
, what many projects do is to determine the version automatically from the git repository information, in particular git tags. setuptools_scm or versioneer are two ways to do this. Using git tags as the basis for versioning is also nice because it makes it easy to go back to the version of the code for older releases. For example here is the code for pvlib version 0.7.1: https://github.com/pvlib/pvlib-python/tree/v0.7.1, which is easy to find because of the git tag.
Cross-reference: openjournals/joss-reviews#4443
When starting GUI.py in Spyder, I have:
ImportError: cannot import name '_missingKeyWarning' from 'bifacial_radiance.main' (C:\Users\Sidi\anaconda3\lib\site-packages\bifacial_radiance\main.py)
Removing it from Lib\site-packages\bifacial_radiance\module.py solves this issue but why? Is _missingKeyWarning really used?
An issue that I mentioned in the Preview stage: the simulations run in the main GUI thread, which make it unresponsive during long time (even if the spyder console output is seen). Simulations should be run in a working thread using the 'threading' library, and a communication between the two threads can be done, e.g. to show the progress of calculations (like a progessbar in tkinter or a simple label). Using threading will make the GUI part remaining responsive and the simulations can be monitored and even stopped when needed. This makes the user experience much more easier.
pvlib and pvfactors are not installed by default in Anaconda, at least in the version I downloaded from the official website.
Therefore I had to install these libs manually using the Anaconda Poweshell prompt:
pip install pvlib
pip install pvfactors
Idem for radiance:
pip install bifacial_radiance
Please update the install procedure accordingly
[openjournals/joss-reviews#4443 (comment)](openjournals/joss-reviews#4443 (comment))
When running BifacialSimu on Mac I get the following error:
/Users/evagrommes/Documents/GitHub/BifacialSimu/BifacialSimu_src/Vendor/pvfactors/geometry/base.py:349: ShapelyDeprecationWarning: Setting custom attributes on geometry objects is deprecated, and will raise an AttributeError in Shapely 2.0
self.list_surfaces = list_surfaces
/Users/evagrommes/Documents/GitHub/BifacialSimu/BifacialSimu_src/Vendor/pvfactors/geometry/base.py:350: ShapelyDeprecationWarning: Setting custom attributes on geometry objects is deprecated, and will raise an AttributeError in Shapely 2.0
self.shaded = self._get_shading(shaded)
/Users/evagrommes/Documents/GitHub/BifacialSimu/BifacialSimu_src/Vendor/pvfactors/geometry/base.py:351: ShapelyDeprecationWarning: Setting custom attributes on geometry objects is deprecated, and will raise an AttributeError in Shapely 2.0
self.is_collinear = is_collinear(list_surfaces)
/Users/evagrommes/Documents/GitHub/BifacialSimu/BifacialSimu_src/Vendor/pvfactors/geometry/base.py:352: ShapelyDeprecationWarning: Setting custom attributes on geometry objects is deprecated, and will raise an AttributeError in Shapely 2.0
self.param_names = param_names
/Users/evagrommes/Documents/GitHub/BifacialSimu/BifacialSimu_src/Vendor/pvfactors/geometry/base.py:937: ShapelyDeprecationWarning: Setting custom attributes on geometry objects is deprecated, and will raise an AttributeError in Shapely 2.0
self.list_segments = tuple(list_segments)
/Users/evagrommes/Documents/GitHub/BifacialSimu/BifacialSimu_src/Vendor/pvfactors/geometry/base.py:938: ShapelyDeprecationWarning: Setting custom attributes on geometry objects is deprecated, and will raise an AttributeError in Shapely 2.0
self._all_surfaces = None
Please add an AUTHORS file to the repo, with the list you gave in Readme.md, with their contribution.
They can also be added in the acknowledgements section.
The installation instructions in the README say to install pvfactors and bifacial_radiance, then edit those installations to use modified files in this repository. This approach seems problematic to me for a few reasons:
pvground.py
and fixes it. BifacialSimu won't get the bugfix because it is overwriting that file with an old version from before the bugfix.If you compare the bifacial_radiance main.py
in this repository with the latest version in their repository (here), there are already many differences. I am worried that using modified files will not be sustainable in the long term, and maybe not even in the short term.
Would it be possible to either get rid of those changes, or to contribute those changes back to the original projects? Maybe there is some other solution to this problem?
Cross-reference: openjournals/joss-reviews#4443
One of the JOSS review checklist items is about software testing:
Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
Having some kind of standard suite of tests has a lot of benefits for scientific software packages, including but not limited to:
What many other packages do, including pvlib, pvfactors, and bifacial_radiance, is have a suite of tests covering most or nearly all of the primary package code. We also run these tests automatically for each commit to give us confidence that we are not accidentally breaking something. Using pvlib as an example:
In my opinion, testing is essentially a requirement for robust and reliable scientific software. For pvlib we take it so seriously that nearly 100% of the package code is covered by tests (see https://app.codecov.io/gh/pvlib/pvlib-python) and we don't accept new code unless it has tests (see pvlib's contributing docs).
Does BifacialSimu have anything along these lines? I didn't see any tests, but maybe I missed it. If not, do you have any plans to implement tests?
Cross-reference: openjournals/joss-reviews#4443
To reproduce:
Full traceback:
Exception in thread Thread-11:
Traceback (most recent call last):
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/site-packages/pandas/core/arrays/datetimes.py", line 2187, in objects_to_datetime64ns
values, tz_parsed = conversion.datetime_to_datetime64(data.ravel("K"))
File "pandas/_libs/tslibs/conversion.pyx", line 359, in pandas._libs.tslibs.conversion.datetime_to_datetime64
TypeError: Unrecognized value type: <class 'str'>
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/threading.py", line 932, in _bootstrap_inner
self.run()
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/threading.py", line 870, in run
self._target(*self._args, **self._kwargs)
File "/home/kevin/projects/BifacialSimu/BifacialSimu_src/GUI.py", line 562, in StartSimulation
Controller.startSimulation(SimulationDict, ModuleDict, resultsPath)
File "/home/kevin/projects/BifacialSimu/BifacialSimu_src/BifacialSimu/Controller/BifacialSimu_simulationController.py", line 91, in startSimulation
df_reportVF, df = BifacialSimu_radiationHandler.ViewFactors.simulateViewFactors(simulationDict, demo, metdata, df, resultsPath, onlyFrontscan = False)
File "/home/kevin/projects/BifacialSimu/BifacialSimu_src/BifacialSimu/Handler/BifacialSimu_radiationHandler.py", line 580, in simulateViewFactors
df['timestamp'] = pd.to_datetime(df['timestamp'])
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/site-packages/pandas/core/tools/datetimes.py", line 887, in to_datetime
values = convert_listlike(arg._values, format)
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/site-packages/pandas/core/tools/datetimes.py", line 401, in _convert_listlike_datetimes
result, tz_parsed = objects_to_datetime64ns(
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/site-packages/pandas/core/arrays/datetimes.py", line 2193, in objects_to_datetime64ns
raise err
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/site-packages/pandas/core/arrays/datetimes.py", line 2175, in objects_to_datetime64ns
result, tz_parsed = tslib.array_to_datetime(
File "pandas/_libs/tslib.pyx", line 379, in pandas._libs.tslib.array_to_datetime
File "pandas/_libs/tslib.pyx", line 611, in pandas._libs.tslib.array_to_datetime
File "pandas/_libs/tslib.pyx", line 749, in pandas._libs.tslib._array_to_datetime_object
File "pandas/_libs/tslib.pyx", line 740, in pandas._libs.tslib._array_to_datetime_object
File "pandas/_libs/tslibs/parsing.pyx", line 257, in pandas._libs.tslibs.parsing.parse_datetime_string
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/site-packages/dateutil/parser/_parser.py", line 1368, in parse
return DEFAULTPARSER.parse(timestr, **kwargs)
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/site-packages/dateutil/parser/_parser.py", line 643, in parse
raise ParserError("Unknown string format: %s", timestr)
dateutil.parser._parser.ParserError: Unknown string format: 11-01 00:00%
I think this line (and a similar one several lines farther down) has an extra %
character:
But even removing that extra %
, the datetime is still not parsed correctly:
Exception in thread Thread-11:
Traceback (most recent call last):
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/site-packages/pandas/core/arrays/datetimes.py", line 2187, in objects_to_datetime64ns
values, tz_parsed = conversion.datetime_to_datetime64(data.ravel("K"))
File "pandas/_libs/tslibs/conversion.pyx", line 359, in pandas._libs.tslibs.conversion.datetime_to_datetime64
TypeError: Unrecognized value type: <class 'str'>
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/threading.py", line 932, in _bootstrap_inner
self.run()
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/threading.py", line 870, in run
self._target(*self._args, **self._kwargs)
File "/home/kevin/projects/BifacialSimu/BifacialSimu_src/GUI.py", line 562, in StartSimulation
Controller.startSimulation(SimulationDict, ModuleDict, resultsPath)
File "/home/kevin/projects/BifacialSimu/BifacialSimu_src/BifacialSimu/Controller/BifacialSimu_simulationController.py", line 91, in startSimulation
df_reportVF, df = BifacialSimu_radiationHandler.ViewFactors.simulateViewFactors(simulationDict, demo, metdata, df, resultsPath, onlyFrontscan = False)
File "/home/kevin/projects/BifacialSimu/BifacialSimu_src/BifacialSimu/Handler/BifacialSimu_radiationHandler.py", line 580, in simulateViewFactors
df['timestamp'] = pd.to_datetime(df['timestamp'])
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/site-packages/pandas/core/tools/datetimes.py", line 887, in to_datetime
values = convert_listlike(arg._values, format)
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/site-packages/pandas/core/tools/datetimes.py", line 401, in _convert_listlike_datetimes
result, tz_parsed = objects_to_datetime64ns(
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/site-packages/pandas/core/arrays/datetimes.py", line 2193, in objects_to_datetime64ns
raise err
File "/home/kevin/miniconda3/envs/bifacialsimu-review/lib/python3.8/site-packages/pandas/core/arrays/datetimes.py", line 2175, in objects_to_datetime64ns
result, tz_parsed = tslib.array_to_datetime(
File "pandas/_libs/tslib.pyx", line 379, in pandas._libs.tslib.array_to_datetime
File "pandas/_libs/tslib.pyx", line 606, in pandas._libs.tslib.array_to_datetime
File "pandas/_libs/tslib.pyx", line 602, in pandas._libs.tslib.array_to_datetime
File "pandas/_libs/tslib.pyx", line 557, in pandas._libs.tslib.array_to_datetime
File "pandas/_libs/tslibs/conversion.pyx", line 516, in pandas._libs.tslibs.conversion.convert_datetime_to_tsobject
File "pandas/_libs/tslibs/np_datetime.pyx", line 120, in pandas._libs.tslibs.np_datetime.check_dts_bounds
pandas._libs.tslibs.np_datetime.OutOfBoundsDatetime: Out of bounds nanosecond timestamp: 1-11-01 00:00:00
It seems to me like the year should be included with %Y
, otherwise how can pd.to_datetime
know what year it is?
Cross-reference: openjournals/joss-reviews#4443
Here are some notes I took while reading the paper.md
manuscript file. Hopefully this feedback is helpful!
PV is the third largest power generation technology and will play a significant role in future global electricity generation (International Renewable Energy Agengy, 2019).
I read this as claiming that solar is currently the third largest power generation technology, which I don't think is what the sentence was intended to convey. Maybe something like "Current forecasts expect PV to be the third largest power generation technology by 2050..." would be better?
There is a need for an open source software tool to simulate bifacial PV, which combines existing radiation and electrical models and provides a graphical user interface (GUI).
I think this deserves more explanation. Why is the combination of viewfactor and raytracing desirable? Why not just use one or the other? I would also argue that albedo should be discussed here. Why are advanced albedo models necessary? I'm not saying they aren't necessary, but that it would strengthen the paper to discuss it.
This comment is a little more vague, but I as the reader am not sure what the main "selling point" of BifacialSimu is, or why I might want to use it instead of some other tool. Here are the questions I'm asking myself as prospective user:
Tied in with this is the question "why create a new tool instead of improving an existing tool?". For example someone could say that it would have been better to add fancy albedo models to bifacial_radiance, and improve its existing gui, instead of making a new wrapper around it. If you can, it would be good to clarify what exactly the need is and why these other existing tools don't address it.
BifacialSimu offers the advantage of an integrated energy yield calculation, whereas bifacial_radiance need a third software, the open-source software PVMismatch [@Mikofski.2018].
I think this is no longer true; my understanding is that bifacial_radiance now includes its own PV I-V calculations using pvlib. See for example their Tutorial 21.
The commonly known open-source software using the viewfactor model, pvfactors [@Anoma.2017] and bifacialVF [@Marion.2017], have both no energy yield calculation and no GUI, which limits the target audience and the application possibilities. The commonly used software PVsyst offers an energy yield calculation and a GUI, but the used physical models are only documented to a limited extent and the software must be paid [@PVsystSA].
NREL's System Advisor Model has a viewfactor-based rear-side irradiance model, does energy yield calculation, has a GUI, is well-documented, and is open source, so if those are the relevant criteria then it's probably worth mentioning SAM here.
I also think BifacialSimu's advanced albedo modeling should be highlighted in this State of the field
section, since that's a clear technical advantage compared to other packages.
Cross-reference: openjournals/joss-reviews#4443
One of the JOSS review checklist items is about instructions for users and contributors:
Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support
I think these points are not really addressed in the README or anywhere else. Here are some examples from other packages if you'd like some inspiration:
Cross-reference: openjournals/joss-reviews#4443
One of the JOSS review checklist items is about dependencies:
Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
I think right now the answer to that question is "no". Looking through the package files, I see that, in addition to the dependencies explicitly mentioned in the README, BifacialSimu requires these external packages:
I think at least some of these are not provided by the default Anaconda distribution, so they should be at least mentioned in the README installation instructions.
However, I agree with the checklist item that it would be better to use an automated package manager to track dependencies. In the python world you can use setup.py
or setup.cfg
for this, which would make your package compatible with tools like pip
. You could also list it on pypi.org if you wanted.
Cross-reference: openjournals/joss-reviews#4443
I ran into two issues with BifacialSimu_src/GUI.py
on linux. The first is that the filepaths for logos, help text, configuration files, etc use hardcoded backslashes, but linux (and Mac OS) use forward slashes, and so I get errors like this:
TclError: couldn't open "/home/kevin/projects/BifacialSimu/BifacialSimu_src\Lib\Button_Images\Button-Info-icon.png": no such file or directory
and
FileNotFoundError: [Errno 2] No such file or directory: '/home/kevin/projects/BifacialSimu/BifacialSimu_src\\Lib\\input_albedo\\Albedo.json'
Editing GUI.py
to use forward slashes gets it working for me, but of course that might break on Windows. To be cross platform you can use os.path.join
or the pathlib
standard library (new in python 3.4).
The second issue is that tkinter doesn't like the .ico
files even after fixing the filepath. For example I get this:
_tkinter.TclError: bitmap "/home/kevin/projects/BifacialSimu/BifacialSimu_src/Lib/Button_Images/info_logo.ico" not defined
But os.path.exists
confirms the file is there, so it must be some incompatibility that I don't understand. Anyway this prevents some things from working, for example the button_SP()
window is empty until I comment out the pop_SP.iconbitmap(...)
line. I did not try to find a better fix since I assume omitting the icons will not break any functionality.
Cross-reference: openjournals/joss-reviews#4443
There are a handful of __pycache__
folders and .DS_Store
files scattered around the repository. Since these are unnecessary and essentially just clutter, I suggest removing them and creating a .gitignore file to prevent them from coming back.
Cross-reference: openjournals/joss-reviews#4443
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.