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

Luis master #96

Merged
merged 37 commits into from
Apr 6, 2022
Merged

Luis master #96

merged 37 commits into from
Apr 6, 2022

Conversation

fredericpoitevin
Copy link
Member

Copy of #57 on main fork, for test.

luis-sribeiro and others added 28 commits January 24, 2022 13:18
The function was generating n_particles^2 coordinates due to an additional for loop.
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov
Copy link

codecov bot commented Apr 6, 2022

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%) ⬆️

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.

@fredericpoitevin fredericpoitevin marked this pull request as ready for review April 6, 2022 08:17
@review-notebook-app
Copy link

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
Copy link

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//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
Copy link

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
Copy link

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
Copy link

View / edit / reply to this conversation on ReviewNB

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

Very nice and clear example! Great work!


@fredericpoitevin
Copy link
Member Author

fredericpoitevin commented Apr 6, 2022

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

@fredericpoitevin
Copy link
Member Author

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

@fredericpoitevin fredericpoitevin merged commit e97e909 into master Apr 6, 2022
@fredericpoitevin fredericpoitevin deleted the luis_master branch April 6, 2022 22:49
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.

2 participants