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

PDD solver: error in the documentation on how to access the output #109

Open
dalonsoa opened this issue May 17, 2020 · 9 comments
Open

PDD solver: error in the documentation on how to access the output #109

dalonsoa opened this issue May 17, 2020 · 9 comments

Comments

@dalonsoa
Copy link
Collaborator

The documentation for the PDD solver states that the output is provided as a nested dictionary. However, that is not correct: it is provided as a State object with the different keys accesible as attributes. For example:

solar_cell[junction_index]['QE']['EQE']

should be:

solar_cell[junction_index].qe.EQE

The examples are incorrect and so is the description of the dictionary at the end.

@Abelarm
Copy link
Contributor

Abelarm commented Oct 17, 2021

I could also take care of this :)

@dalonsoa
Copy link
Collaborator Author

Please, go ahead! Please, let me know if after making these changes, something is still not working or is not accurate.

@Abelarm
Copy link
Contributor

Abelarm commented Oct 19, 2021

I am having a problem with the PDD, I have installed everything with the “with_pdd” no problem but keep having the warning about the ddModel could not be imported

@dalonsoa
Copy link
Collaborator Author

Have a look at this #168 (comment)

It should be a solved problem in the current master and develop branches. It might be you need to update?

@Abelarm
Copy link
Contributor

Abelarm commented Oct 19, 2021

I already have that version the point is that I do not have file "ddModel.cp39-win_amd64.pyd"

@dalonsoa
Copy link
Collaborator Author

That's funny. It does work with Windows and Python 3.9 in the CI system...

Could you run f2py manually in the poisson_drift_diffusion folder to compile the fortran library? Or maybe uninstall and then re-install again Solcore in edit mode but maybe commenting out this line, so you see if there is any error.

@Abelarm
Copy link
Contributor

Abelarm commented Oct 19, 2021

Great I run this command:
python -m numpy.f2py DDmodel-current.f95-m ddModel -h ddModel.pyf

and it created a .so and a .pyf and now it works!!

@Abelarm
Copy link
Contributor

Abelarm commented Oct 20, 2021

I am not really sure what to fix here:

1 All the examples apart the last one works fine.

2 For the second example the Bandstructure i cannot access as a State (is a dict in-fact)
image

3 For the second example I was able to change my_solar_cell[0].recombination_currents['Jrad'] to my_solar_cell[0].recombination_currents.Jrad (as example) and it works

3b For the second part of the example I get an error for this line, I would change it be be a f-string instead of format

plt.text(0, 200, 'Voc = {:4.1f} V\n'
                 'Isc = {:4.1f} A/m${^2}$\n'
                 'FF = {:4.1f} %\n'
                 'Pmpp = {:4.1f} W/m${^2}$'.format(my_solar_cell.iv['Voc'], my_solar_cell.iv['Isc'],
                                           my_solar_cell.iv['FF'] * 100, my_solar_cell.iv['Pmpp']))

4 For the last example tries to access: my_solar_cell[0].qe_data.wavelengths which does not exist in the State even when changing to my_solar_cell[0].qe.wavelengths

So it's a mix of several things here :/

@dalonsoa
Copy link
Collaborator Author

I'm sorry it took me so long to answer. Yes, this is really disconcerting and, honestly, very confusing for the users being the output so inconsistent.

I think that the only solution - and is not really a solution - that does not break backwards compatibility is to go through all the relevant functions in the PDD solver and document exactly what their outputs are, event if these are weird and inconsistent.

Eventually, we need to put some order here and refactor all the outputs, so things make sense, but I don't think we are there, yet, I am afraid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants