compSPI / simSPI

Generative Models
9 stars 10 forks source link

Luis master #96

Closed fredericpoitevin closed 2 years ago

fredericpoitevin commented 2 years ago

Copy of #57 on main fork, for test.

review-notebook-app[bot] commented 2 years ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

codecov[bot] commented 2 years ago

Codecov Report

Merging #96 (bd48fd5) into master (b66f479) will increase coverage by 0.27%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #96      +/-   ##
==========================================
+ Coverage   98.12%   98.39%   +0.27%     
==========================================
  Files          15       15              
  Lines         744      744              
==========================================
+ Hits          730      732       +2     
+ Misses         14       12       -2     
Impacted Files Coverage Δ
simSPI/tem.py 97.55% <0.00%> (+1.64%) :arrow_up:

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 dcd6f71...bd48fd5. Read the comment docs.

review-notebook-app[bot] commented 2 years ago

View / edit / reply to this conversation on ReviewNB

geoffwoollard commented on 2022-04-06T15:14:05Z ----------------------------------------------------------------

"In order to run the tests, it is also necessary to have installed the TEM-simulator" --> Modify text to reflect containerization.


review-notebook-app[bot] commented 2 years ago

View / edit / reply to this conversation on ReviewNB

geoffwoollard commented on 2022-04-06T15:14:06Z ----------------------------------------------------------------

Is there any docs in the TEM simulator on what convention is used? It would be really good to show how these angles are applied in the projection. In case this is out of scope for this PR, I have created an issue here: https://github.com/compSPI/simSPI/issues/97

  1. Read in angles from the output data
  2. Make a rotation matrix, according to the right Euler angle convention: e.g. pytorch3d.transforms.euler_angles_to_matrix(..., convention='XZX')
  3. Use the linear simulator to rotate and project
  4. Show visually/numerically that the projections are identical. Numerically might be a bit tricky if the centring is off somehow in the TEM simulator. If so, visually could suffice.

review-notebook-app[bot] commented 2 years ago

View / edit / reply to this conversation on ReviewNB

geoffwoollard commented on 2022-04-06T15:14:07Z ----------------------------------------------------------------

Could there be a bit more explanation of why these settings give a "perfect" micrograph? Is it one or more of the settings below? If there is no defocus, and the cs is zero then the CTF should be sin(0), no? Or is there some amplitude contrast that contributes a cos(0) = 1 part, making the CTF = Asin(0) + Bcos(0) = B?

I don't see any thing for A and B or some ratio of them (amplitude contrast) in the settings.

See Eq 1 in the CTFFIND4 (2015) paper for ref: 10.1016/j.jsb.2015.08.008

=== optics ===

magnification = 81000

cs = 0.0

cc = 0.0

aperture = 50

focal_length = 3.5

cond_ap_angle = 0.1

gen_defocus = yes

defocus_nominal = 0.0

defocus_syst_error = 0.0

defocus_syst_error = 0.0

defocus_file_out = None


review-notebook-app[bot] commented 2 years ago

View / edit / reply to this conversation on ReviewNB

geoffwoollard commented on 2022-04-06T15:14:08Z ----------------------------------------------------------------

interesting circular artefact in the middle panel... perhaps it's from one of the aperture settings in the TEM simulator.


review-notebook-app[bot] commented 2 years ago

View / edit / reply to this conversation on ReviewNB

geoffwoollard commented on 2022-04-06T15:14:08Z ----------------------------------------------------------------

Very nice and clear example! Great work!


fredericpoitevin commented 2 years ago

Closes #57 since it is essentially the same with some fixes.

fredericpoitevin commented 2 years ago

I opened #98 to address the fact that the "Building simSPI Container" check is skipped.