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

Solve issue with boundary condition in depletion approximation #253

Merged
merged 10 commits into from Apr 6, 2023

Conversation

phoebe-p
Copy link
Member

@phoebe-p phoebe-p commented Mar 31, 2023

See issue #252.
Believe the problem is solved in the solve_bvp (default) method, but not in the Green's function method. This should happen before merging.

Edit: I believe the issue is now also solved in the Green's function method.

Other changes:

  • Incorporate the devpy fixes (pinning to version 0.1) from Pin devpy and update commands to v0.1 API #251
  • Changed how the Green's function method is called in iv_depletion to avoid looping over wavelengths (instead pass the wavelength-integrated generation*incident flux directly to the Green's function solver, similar to what already happens for the solve_bvp method). This makes the calculation for IV much faster.
  • Changed the default DA method to 'green'. I don't see any reason not to since the results should be identical, I think the solver is probably more robust, and it's faster.
  • Updated the MJ_solar_cell_using_DA example to be more up-to-date: using TMM with a defined ARC instead of loading reflectance from an external file, and using GaAs rather than loading InGaAs data from an external file (until issue InGaAs material data is incorrect #255 is fixed). The previous version of the example has been renamed MJ_solar_cell_external_reflectance, since it is still useful to have a demonstration of that functionality.
  • There was a bug in the interpolation of the TMM and RCWA absorption profiles (this was especially noticeable if the mesh used in the DA solver was finer than what was used in the optics calculation). Previously:
def calculate_absorption_tmm(tmm_out, initial=1):
    all_z = tmm_out["position"] * 1e-9
    all_abs = initial * tmm_out["absorption"] / 1e-9

    def diff_absorption(z):
        idx = all_z.searchsorted(z)
        idx = np.where(idx <= len(all_z) - 2, idx, len(all_z) - 2)
        try:
            z1 = all_z[idx]
            z2 = all_z[idx + 1]

            f = (z - z1) / (z2 - z1)

            out = f * all_abs[:, idx] + (1 - f) * all_abs[:, idx + 1]

        except IndexError:
            out = all_abs[:, idx]

        return out

    all_absorbed = np.trapz(diff_absorption(all_z), all_z)

    return diff_absorption, all_absorbed

Due to how searchsorted works, this meant both z1 and z2 were actually lower than z, which doesn't make sense and gives negative values for f, and could in turn give negative values for the generation profile.

stefanv and others added 2 commits March 14, 2023 13:58
Please note that `devpy` will soon be renamed to `spin` due to PyPi
name availability.
@codecov
Copy link

codecov bot commented Apr 3, 2023

Codecov Report

Merging #253 (05c50d5) into develop (f476648) will decrease coverage by 0.06%.
The diff coverage is 82.77%.

❗ Current head 05c50d5 differs from pull request most recent head 02045e1. Consider uploading reports for the commit 02045e1 to get more accurate results

@@             Coverage Diff             @@
##           develop     #253      +/-   ##
===========================================
- Coverage    57.12%   57.07%   -0.06%     
===========================================
  Files          103      103              
  Lines        11621    11633      +12     
===========================================
  Hits          6639     6639              
- Misses        4982     4994      +12     
Impacted Files Coverage Δ
solcore/optics/rcwa.py 31.08% <0.00%> (-3.17%) ⬇️
solcore/absorption_calculator/transfer_matrix.py 66.19% <68.29%> (-0.01%) ⬇️
...re/analytic_solar_cells/depletion_approximation.py 100.00% <100.00%> (ø)
solcore/optics/tmm.py 89.47% <100.00%> (+0.14%) ⬆️

... and 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@iclned
Copy link
Collaborator

iclned commented Apr 5, 2023

Several issues have been addressed that were causing unexpected behaviour with EQE calculations. The new MJ example is much clearer. Performance of light-IV is improved.

@phoebe-p phoebe-p merged commit 4f1b2cf into develop Apr 6, 2023
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants