Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add spectral factor gallery example #2114

Merged
merged 40 commits into from
Jul 9, 2024
Merged

Conversation

RDaxini
Copy link
Contributor

@RDaxini RDaxini commented Jul 1, 2024

  • Closes add spectrum.spectral_factor example #2107
  • I am familiar with the contributing guidelines
  • 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`).
  • New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
  • Pull request is nearly complete and ready for detailed review.
  • Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

Context described in Issue #2107

Docs here


Outdated:

Current questions I'd still like to ask:

  • What duration of analysis is best? I analyse a week and plot both one week and a day at present.
  • If sticking with the current duration, I prefer to plot the full week and then zoom in on one day; but this section of code:
df_results.index = m_fs.index
df_results = df_results.loc['2001-08-01':'2001-08-01']
df_results.index = df_results.index.strftime("%H:%M")
fig2, ax1 = plt.subplots()
df_results.plot(ax=ax1)
plt.show()

where I effectively reset and then reformat the index, and then replot a new graph seems inefficient. Is it possible to do this in a better way? Somehow duplicating and limiting the axis of the first graph perhaps?

  • For one day, between what times should data be plotted? Starting from 0600 enables the inclusion of a value of M generated by the First Solar Model when the other two modules yield 0, but this means the y-axis must start from zero, which cramps up the plots to the point where it is hard to visualise differences between the curves. Starting at 0700 resolves this but means that one non-zero value of M is omitted. PV output at these times is likely negligible but for the purpose of illustrating a calculation of M using different models I am still wondering what might be the best option here.

@RDaxini RDaxini changed the title Add spectral factor gallery example Add spectral factor gallery example (Draft) Jul 1, 2024
@kandersolar kandersolar added this to the v0.11.1 milestone Jul 2, 2024
@kandersolar
Copy link
Member

What duration of analysis is best? I analyse a week and plot both one week and a day at present.

That sounds reasonable to me!

I effectively reset and then reformat the index

If the default formatting is not sufficient, I suggest leaving the index of the data as-is and applying formatting to the plot rather than the data. But I'll say that I rarely find myself displeased with the default timestamp formatting, so I wonder if it's really needed in this case.

Is it possible to do this in a better way?

I would just subset the dataframe for the second plot. Something like df.loc['2022-06-02'].plot(ax=ax) if I want a single day.

For one day, between what times should data be plotted?

I suggest performing the simulation for the full time series (including the dawn/dusk hours and even nighttime), and just setting the y-axis limits to zoom into the region of interest. Something like plt.ylim(0.95, 1.05).

@RDaxini
Copy link
Contributor Author

RDaxini commented Jul 3, 2024

Thank you for this feedback @kandersolar
I will work on these points as advised and then mark the PR ready for review.

@RDaxini RDaxini marked this pull request as ready for review July 3, 2024 13:42
@RDaxini RDaxini changed the title Add spectral factor gallery example (Draft) Add spectral factor gallery example Jul 3, 2024
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
RDaxini and others added 2 commits July 3, 2024 15:13
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
@kandersolar kandersolar added the GSoC Contributions related to Google Summer of Code. label Jul 3, 2024
RDaxini and others added 8 commits July 3, 2024 15:58
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
Copy link
Contributor

@IoannisSifnaios IoannisSifnaios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @RDaxini! Some ideas from my side...

docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
RDaxini and others added 3 commits July 4, 2024 15:22
Co-authored-by: Ioannis Sifnaios <88548539+IoannisSifnaios@users.noreply.github.com>
Co-authored-by: Ioannis Sifnaios <88548539+IoannisSifnaios@users.noreply.github.com>
Copy link
Member

@AdamRJensen AdamRJensen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the differentiation between spectral factor and spectral mismatch is unclear, can this somehow be explained in the first paragraph

docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/sphinx/source/whatsnew/v0.11.1.rst Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
Copy link
Contributor

@echedey-ls echedey-ls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments below; the example is easy to follow, I like it.

docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Show resolved Hide resolved
docs/examples/spectrum/spectral_factor.py Show resolved Hide resolved
Co-authored-by: Echedey Luis <80125792+echedey-ls@users.noreply.github.com>
Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
@RDaxini
Copy link
Contributor Author

RDaxini commented Jul 8, 2024

Good to go now from my side.
Let me know if there any additional changes that you'd like to see. Thanks for all of the reviews so far.

docs/examples/spectrum/spectral_factor.py Outdated Show resolved Hide resolved
@kandersolar kandersolar merged commit b6c795d into pvlib:main Jul 9, 2024
30 checks passed
@kandersolar
Copy link
Member

Thanks @RDaxini!

@RDaxini RDaxini deleted the scf_example branch July 9, 2024 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation GSoC Contributions related to Google Summer of Code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add spectrum.spectral_factor example
6 participants