cmelab / morphct

GNU General Public License v3.0
0 stars 6 forks source link

Feat/pyscf #3

Closed jennyfothergill closed 3 years ago

jennyfothergill commented 3 years ago

This is still very WIP, but I want to keep track of some assumptions I am making in the simplification of this code so I don't forget.

  1. I am paring down functionality (for now at least) to only calculate mobility (no coarse graining/fine graining/ device stuff)
  2. I am switching orca for pySCF
  3. The only input now will be an atomistic gsd snapshot.
  4. It is assumed the snapshot lengths are in units of Angstoms. ~(I need to add a way to scale them to make this true) TODO~
  5. ~I'm noticing the displacement is only calculated after all hops are finished so if a carrier goes from chromophore 0->1->2->0 it's displacement is zero. Should we fix this?~
codecov[bot] commented 3 years ago

Codecov Report

Merging #3 (ef6101c) into master (eeca318) will increase coverage by 85.85%. The diff coverage is 85.88%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master       #3       +/-   ##
===========================================
+ Coverage    0.03%   85.88%   +85.85%     
===========================================
  Files          22        8       -14     
  Lines        6190      751     -5439     
===========================================
+ Hits            2      645      +643     
+ Misses       6188      106     -6082     
Impacted Files Coverage Δ
morphct/kmc_analyze.py 77.04% <77.04%> (ø)
morphct/helper_functions.py 84.52% <84.52%> (ø)
morphct/execute_qcc.py 85.03% <85.03%> (ø)
morphct/mobility_kmc.py 90.15% <90.15%> (ø)
morphct/chromophores.py 93.80% <93.80%> (ø)
morphct/__init__.py 100.00% <100.00%> (ø)
morphct/__version__.py 100.00% <100.00%> (ø)
morphct/transfer_integrals.py 100.00% <100.00%> (ø)
morphct/tests/test_imports.py
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update eeca318...ef6101c. Read the comment docs.

jennyfothergill commented 3 years ago

Tests are taking a long time to run I think because of (memory leak? trouble linking with c libraries? numpy incompatibility?) with pyscf. Will look into tomorrow.

jennyfothergill commented 3 years ago

tests are passing again. The error had to do with difference in the way multiprocessing sets its context on OSX (lappy) and linux (the container). Adding a "spawn" context wrapper in the functions using multiprocessing.Pool fixed them hanging forever. c:

It was definitely fun spending hours going down a rabbit hole thinking the problem was blas library incompatibilities with pyscf. 😆

erjank commented 3 years ago

Re: I'm noticing the displacement is only calculated after all hops are finished so if a carrier goes from chromophore 0->1->2->0 it's displacement is zero. Should we fix this?

This should be the expected behavior, assuming it's implemented correctly: If a charge ends up where it started after a change in time, its displacement IS zero, and if lots of charges end up trapped on a subnetwork, we want that to be reflected in our measurement. Otherwise, charges zipping around in circles would count toward a high charge mobility, when our network has zero net conductivity.

jennyfothergill commented 3 years ago

I need to make it easier to scale the distances in the gsd file and be more explicit about what units pyscf expects (by default Angstrom https://pyscf.org/user/gto.html )

erjank commented 3 years ago

yeah

Eric Jankowski PhD | he/him/his Associate Professor | Micron School of Materials Science & Engineering Boise State University | 1910 W University Dr. | Boise, ID 83725-2090 @.*** | (208) 426-5681 Zoom: https://boisestate.zoom.us/j/3783842550 Book my calendar: https://ericjankowski.youcanbook.me/

On Tue, Jun 8, 2021 at 1:55 PM Jenny @.***> wrote:

@.**** commented on this pull request.

In morphct/chromophores.py https://github.com/cmelab/morphct/pull/3#discussion_r647750544:

  • Energy required to "reorganize" the system structure from the final to
  • initial coordinates. (what paper did this default come from? TODO EJ)
  • vrh_delocalization : float, default 2e-10
  • Variable-range hopping (rate? where did default come from?) TODO EJ
  • Attributes

  • id : int
  • Index of this chromophore in the system.
  • species : str
  • Chromophore species ('donor' or 'acceptor')
  • reorganization_energy : float
  • Energy required to "reorganize" the system structure from the final to
  • initial coordinates.
  • vrh_delocalization : float
  • Variable-range hopping (rate?) TODO EJ

OK so the vrh_delocalization value is in the same units as the distances?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cmelab/morphct/pull/3#discussion_r647750544, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIJRSWEXFCOTFFW4GJJTMDTRZYR7ANCNFSM4X5GNIXA .

jennyfothergill commented 3 years ago

Thanks to @JimmyRushing for finding another potential issue