pvlib-python icon indicating copy to clipboard operation
pvlib-python copied to clipboard

add mlfm

Open steve-ransome opened this issue 2 years ago • 41 comments

  • [x] Closes #925
  • [x] I am familiar with the contributing guidelines
  • [x] Tests added
  • [ ] Updates entries to docs/sphinx/source/api.rst for API changes.
  • [ ] Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/whatsnew for all changes. Includes link to the GitHub Issue with :issue:`num` or this Pull Request with :pull:`num`. Includes contributor name and/or GitHub username (link with :ghuser:`user`).
  • [x] New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
  • [x] Pull request is nearly complete and ready for detailed review.
  • [x] Maintainer: Appropriate GitHub Labels and Milestone are assigned to the Pull Request and linked Issue.

steve-ransome avatar Dec 10 '21 17:12 steve-ransome

@wholmgren @kanderso-nrel would appreciate some advice here. As currently organized, #1354 adds two python modules to pvlib-python: mlfm.py contains the parameter estimation and model functions, and mlfm_graphs.py contains code to produce figures which are important to the model fitting and model evaluation processes. mlfm_graphs.py imports matplotlib which is not part of pvlib's requirements. @steve-ransome that's why the tests are failing.

What makes sense?

  • add matplotlib to requirements. I feel like we'd regret this but I can't articulate why.
  • move the plotting functions to the tutorial code. Users could get the code but to using it would involve copying and pasting etc. Not the most convenient.
  • exclude mlfm_graphs.py from testing.

I'm not enamored with both pvlib.mlfm and pvlib.mlfm_graphs but I don't have a better idea atm.

cwhanse avatar Dec 13 '21 15:12 cwhanse

I've not yet done more than a quick skim of this code, so take these as general comments.

exclude mlfm_graphs.py from testing.

RdTools has a plotting module. We decided that it would be too brittle to test those functions beyond (1) does it run without error and (2) does it return a Figure. Examples of more complete testing strategies that seem too brittle: inspect the Figure object attributes; save a PNG and compare pixels with a reference image.

add matplotlib to requirements. I feel like we'd regret this but I can't articulate why.

FWIW requiring matplotlib has not really been a technical burden in other projects I work on. Including plotting functions in general might be a pain because everyone has their own favorite plotting package; for example, RdTools uses matplotlib for some functions and plotly for others.

I'm not enamored with both pvlib.mlfm and pvlib.mlfm_graphs

Could have pvlib.mlfm be a folder with an __init__.py and separate submodules, the same way #717 is (re)structuring pvlib.bifacial.

One other note: I wonder if mlfm would work better as a separate repo, i.e. pvlib/mlfm instead of pvlib.mlfm. If looking at plots is important and there isn't much (any?) integration with the rest of pvlib-python, maybe it's more of an application than a component of a modeling chain?

kandersolar avatar Dec 13 '21 17:12 kandersolar

I wonder if mlfm would work better as a separate repo

Personally, I think it would, but that is very unlikely to happen. I'm aware of at least one commercial pvlib user who is also using the mlfm (as private code). We discussed pros and cons of including it in pvlib here

cwhanse avatar Dec 13 '21 18:12 cwhanse

Thanks for the comments.

The MLFM can do many tasks including

  • fitting and analysing IV curves (comparing with desoto etc)
  • apportioning loss mechanisms and limits
  • derives low light and temperature coefficients more easily than desoto or SAPM.
  • fitting indoor or outdoor measurement data (including IEC 61853) and can be compared with desoto and SAPM
  • predicting PR, power, voltage or current vs. irradiance and temperature for energy yield predictions (as sapm)
  • derive degradation rates and causes versus irradiance and temperature (e.g. shunt resistance fall, worse at low light than high light) and can be compared with other models.

To keep this code short I haven't shown any integration with the rest of pvlib here but have been using it with pvlib weather functions, angles of incidence, tmy 2 power calculations and inverters since at least 2015 and have published a few papers with Sandia including http://www.steveransome.com/PUBS/502_42pvsc_NewOrleans_150610t12.pdf and https://pvpmc.sandia.gov/download/7879/

Although the MLFM uses graphics to highlight its findings (I used matplotlib because all the tutorials did) it does not need to, all of the graphs come from calculations stored in pandas dataframes.

My feeling is it should be integrated with pvlib and I will do whatever I can to reduce any problems with graphics.

steve-ransome avatar Dec 14 '21 14:12 steve-ransome

I don't have strong feelings about it, but at the moment I prefer

I know discussion in #925 suggested multiple files in a subpackage but to me that API seems unnecessarily complex.

In general I support the idea of including functions for standardized, high quality graphics in pvlib. Needs to be a lot more than df.plot() e.g. @AdamRJensen's solar_multiplot

wholmgren avatar Dec 14 '21 15:12 wholmgren

@steve-ransome I will push some changes to implement @wholmgren's suggestions.

cwhanse avatar Dec 14 '21 16:12 cwhanse

@steve-ransome sorry I set this aside for a while. Next step: we need to write tests for each function that is being added. For numerical functions, can you code a set of inputs with known output and put them in a new file tests/test_mlfm.py? We can re-use these data to test the plot functions.

cwhanse avatar Jan 07 '22 02:01 cwhanse

Thanks Cliff. I looked through some of the existing tests to get an idea what was needed, there were several different styles/things included.

This file should run and check three functions with one date_time. If anyone has comments I will follow them then put in the repository.

Should I write one for mlfm_fit as this will need many date_times (probably 20 would do).

That might be enough to check the graph functions - if you wish me to do that

test_mlfm_220111t17.py.txt .

steve-ransome avatar Jan 11 '22 17:01 steve-ransome

@steve-ransome if you can develop a test for mlfm_fit I'll go ahead and organize that python script into test_mlfm. I'm still unsure how to test the plotting functions; perhaps we only check that the functions return a figure and leave it at that.

cwhanse avatar Jan 11 '22 18:01 cwhanse

Here's an update with a test for mlfm_fit using the matrix data from mlfm.ipynb selecting option 2. I think although we can get calculations with tolerance_calc atol=0.0000001 we might only get fits to be tolerance_fit atol=0.001 or so as the Excel and Python RMSE only differ in the 5th significant figure 0.0027961 vs. 0.0027962

test_mlfm_220114t17.py.txt

If we wanted to test then graphics then the mlfm.ipynb with file option 2 gives these graphs, we could check that they plot and look ok.

mlfm

steve-ransome avatar Jan 14 '22 18:01 steve-ransome

I'm thinking of submitting a paper on this work for WCPEC8 . It would be a little about the mlfm and code but mostly highlighting PVLIB and the methods and procedures I had to go through to get it up to standard and included, bearing in mind that I have no programming qualifications and little experience in collaborative work so hadn't used git hub etc. Any thoughts? I'd obviously provide all the acknowledgements etc. for the conference and give links etc to help anyone else contribute their work.

steve-ransome avatar Jan 30 '22 18:01 steve-ransome

I have now written an abstract paper called "ADDING THE MLFM TO PVPMC/PVLIB" and intend to submit it to WCPEC8 this evening European time 4th Feb (the deadline). It will have lots of links to PVPMC. If anyone wishes to make any comments before I submit it, please do. If it's accepted I would obviously interact more and could add some extra authors.

steve-ransome avatar Feb 04 '22 10:02 steve-ransome

"ADDING THE MLFM TO PVPMC/PVLIB" was accepted as a poster at WCPEC8. It would be great to get some feedback for this, I can put the abstract up here if you wish. All contributions will be acknowledged or I could add people as joint authors.

steve-ransome avatar May 10 '22 13:05 steve-ransome

@kanderso-nrel what is the status of pvlib-python\docs\tutorials? I'm not seeing them on the new documentation pages; are they retired, or am I missing the link?

cwhanse avatar May 19 '22 19:05 cwhanse

Other than a brief mention on the matlab comparison page I don't think the notebooks in docs/tutorials have ever been integrated with the readthedocs sphinx pages. I usually forget those notebooks exist and if I had to guess I'd speculate that most users don't know about them. I don't think we've officially retired them, but they do tend to collect dust and get out of date. Maybe we should just merge their content into the readthedocs User Guide pages and get rid of the notebooks.

For this PR I wouldn't object to tossing mlfm.ipynb onto the docs/tutorials pile and sorting out what to do with the notebooks later. But I doubt many people will find it in there in the meantime.

kandersolar avatar May 19 '22 20:05 kandersolar

But I doubt many people will find it in there in the meantime.

A paper referring and linking to it should help.

adriesse avatar May 20 '22 09:05 adriesse

@steve-ransome I have the three tutorial notebooks cleaned up and running.

There are some parts of mlfm functions that are not hit by the tests. Could you take a look at providing data for test cases that hit mlfm_norm_to_stack lines 271-294. Here, we need a normalized data set that has 'v_mp' and 'i_mp' instead of 'v_ff', 'r_oc', 'i_ff', 'r_sc'. The same data will improve test coverage for plot_mlfm_stack.

cwhanse avatar Jun 30 '22 22:06 cwhanse

Thanks Cliff, I am looking at this now to try to find the easiest way.

If we have 6 mlfm variables then the 4 variables derive straight from them, i_sc and v_oc are unchanged and i_mp =i_ff * r_sc and v_mp = v_ff * r_oc

If we check the 6 (i_sc, r_sc, i_ff, v_ff, r_oc, v_oc) and it works, so must the 4 (i_sc, r_sc * i_ff, v_ff * r_oc, v_oc).

Do you think we still need to create and test the 4 numbers separately?

steve-ransome avatar Jul 01 '22 17:07 steve-ransome

Do you think we still need to create and test the 4 numbers separately?

I do, so that the tests touch all lines of code which do computations. Sounds like we can make a 4 variable test from the existing 6 variable test data?

cwhanse avatar Jul 01 '22 18:07 cwhanse

Exactly. We don't need new measurement data of a 4 variable set, we can convert 6 --> 4 and use that
as i_mp =i_ff * r_sc and v_mp = v_ff * r_oc.

steve-ransome avatar Jul 01 '22 18:07 steve-ransome

@pvlib/pvlib-maintainer I think this is ready for a look. Coverage is a bit shy because the tests don't hit all of the try/except blocks.

cwhanse avatar Jul 12 '22 01:07 cwhanse

@cwhanse I suspect the windows test cancellations are because of a matplotlib window blocking the thread. Telling matplotlib to use non-interactive graphics by adding matplotlib.use('agg') to conftest.py seems to fix it for me locally. See https://github.com/matplotlib/matplotlib/issues/22616

kandersolar avatar Jul 12 '22 13:07 kandersolar

I'll be presenting a poster "3BV3 WCPEC #8 ADDING THE MLFM TO PVPMC/PVLIB" at WCPEC8. Basically it summarises the flow chart, graphs and some code.

If any PVLIB guru is around at the session it would be great if they could see me there if there are pvlib questions I can't answer.

The poster has a section (cut and pasted below) on how to contribute code with lots of links. I paraphrased this from the latest pvlib page https://pvlib-python.readthedocs.io/en/stable/contributing.html#documentation

Please let me know if you have any comments, additions or corrections. I can send a draft poster now or can forward it the final version before I get it printed.

Is there an official QR code to use? I've added the "Powered by PVLIB" logo. If anyone wants to leave some stickers at the poster leave them with me and I can hand them out.

Thanks in advance.

" How to contribute to PVLIB (add a QR code?)

INFORMATION : Stack Overflow : http://stackoverflow.com/questions/tagged/pvlib Google groups : https://groups.google.com/forum/#!forum/pvlib-python GitHub issues : https://github.com/pvlib/pvlib-python/issues Pull requests : https://github.com/pvlib/pvlib-python/pulls Jupyter Notebook tutorials : https://github.com/pvlib/pvlib-python/tree/master/docs/tutorials

Follow PEP8 : https://www.python.org/dev/peps/pep-0008/. (Max line length 79 chars) Add your project : https://github.com/pvlib/pvlib-python/wiki/Projects-and-publications-that-use-pvlib-python. Variables and Symbols : https://pvlib-python.readthedocs.io/en/stable/user_guide/variables_style_rules.html#variables-style-rules Documentation style : https://pvlib-python.readthedocs.io/en/stable/contributing.html#documentation 

Coding : PVSystem and Location classes provide convenience wrappers around the core pvlib functions. Remove logging calls and print statements. Include documentation and Comprehensive unit tests Functions must return the desired output for all inputs (see https://github.com/pvlib/pvlib-python/issues/394).

Code of conduct : https://github.com/pvlib/pvlib-python/blob/master/CODE_OF_CONDUCT.md"

steve-ransome avatar Sep 05 '22 16:09 steve-ransome

I'll be presenting a poster "3BV3 WCPEC #8 ADDING THE MLFM TO PVPMC/PVLIB" at WCPEC8. Basically it summarises the flow chart, graphs and some code. If any PVLIB guru is around at the session it would be great if they could see me there if there are pvlib questions I can't answer.

I will unfortunately not be at WCPEC as I'm attending EuroSun which occurs simultaneously - maybe @adriesse will be?

Is there an official QR code to use? I've added the "Powered by PVLIB" logo. If anyone wants to leave some stickers at the poster leave them with me and I can hand them out.

Sadly, all of the stickers reside with Silvana or me - I suppose I should have done more to spread them out...

AdamRJensen avatar Sep 06 '22 04:09 AdamRJensen

@adriesse is due to chair a session with me at wcpec8 so I hope he'll be there. You could always mail some to me or him. Let me know if you want my address in the UK.

steve-ransome avatar Sep 06 '22 05:09 steve-ransome

I'll be there! Adam, could you bring some stickers to Freiburg the week before?

adriesse avatar Sep 06 '22 07:09 adriesse

@adriesse I sadly will only attend our IEA PVPS Task 16 virtually this time, as I will be returning from my stay at NREL the same day.

AdamRJensen avatar Sep 08 '22 18:09 AdamRJensen

I presented my poster at WCPEC8 today in Milano.

There was a lot of interest, not only in the MLFM but also using pvlib and also several people said they'd like to add their code.

This is the poster that I will submit (without the "to be presented" box). Any comments gratefully received.

2209_WCPEC8_Milan_poster_pvpmc.pdf

steve-ransome avatar Sep 27 '22 19:09 steve-ransome

Thank you, Steve. You and I will likely need to get on another Zoom call to address @adriesse comments.

cwhanse avatar Sep 27 '22 20:09 cwhanse

Here's a "powered by PVLIB" sticker in the wild. image

steve-ransome avatar Sep 29 '22 20:09 steve-ransome