CoM rotation and flip in the GUI #1087

sk1p · 2021-08-23T13:49:23Z

Fixes #31

TODO

The template for notebook download should also be updated to include the new parameters.
Fix results data download.
Add better input method (slider) in addition to allowing to enter values directly
- Fix for easily entering negative rotation angles in number input
Add documentation
- CoM section in applications section → @uellue will take care of this one
- Explain parameters (info box or similar?)
Fix bug: time delta resets after analysis finishes

Contributor Checklist:

I have added or updated my entry in the creators.json file
I have added a changelog entry for my contribution
I have added/updated documentation for all user-facing changes
I have added/updated test cases
I have included the rebuilt production build of the client (only if changes were made to the GUI)

Reviewer Checklist:

/azp run libertem.libertem-data passed

+ `Analysis.need_rerun`: based on old and new parameters, decide if we can re-use an existing UDF result, or if we need to re-run + `COMAnalysis`: don't re-run if only `flip_y` or `scan_rotation` changed + JobDetailHandler: re-use existing UDF results, based on `Analysis.need_rerun`

codecov · 2021-08-23T13:52:19Z

Codecov Report

Merging #1087 (7f54f8a) into master (7c2d502) will increase coverage by 0.15%.
The diff coverage is 57.14%.

@@            Coverage Diff             @@
##           master    #1087      +/-   ##
==========================================
+ Coverage   69.28%   69.44%   +0.15%     
==========================================
  Files         268      268              
  Lines       12614    12795     +181     
  Branches     1777     1817      +40     
==========================================
+ Hits         8740     8885     +145     
- Misses       3532     3567      +35     
- Partials      342      343       +1

Impacted Files	Coverage Δ
...mpoundAnalysis/components/CenterOfMassAnalysis.tsx	`0.00% <0.00%> (ø)`
...alysis/components/layouts/AnalysisLayoutTwoCol.tsx	`0.00% <ø> (ø)`
client/src/compoundAnalysis/helpers.ts	`0.00% <0.00%> (ø)`
client/src/messages.ts	`100.00% <ø> (ø)`
src/libertem/api.py	`93.06% <ø> (ø)`
src/libertem/analysis/disk.py	`94.00% <50.00%> (-1.84%)`	⬇️
src/libertem/web/base.py	`89.36% <50.00%> (ø)`
src/libertem/analysis/base.py	`95.29% <100.00%> (+0.11%)`	⬆️
src/libertem/analysis/com.py	`96.66% <100.00%> (+0.27%)`	⬆️
src/libertem/io/writers/results/base.py	`100.00% <100.00%> (ø)`
... and 6 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 7c2d502...7f54f8a. Read the comment docs.

uellue

Looks good so far! We should include a link or info sign that explains the parameters. We have them documented in https://libertem.github.io/LiberTEM/reference/api.html#libertem.api.Context.create_com_analysis, for example. :-)

With near-instantaneous recalculation after parameter change it would be awesome to have a slider for the angle, or a rotation handle on the widget for position and size!

I noticed that we don't have COM in our Applications section. We could perhaps create a stub for the "phase contrast" application with an API example, link to reference docs and one screenshot for COM, and a link to https://ptychography-4-0.github.io/ptychography/algorithms.html for SSB ptychography?

.editorconfig

src/libertem/analysis/com.py

uellue · 2021-08-23T15:42:51Z

The template for notebook download should also be updated to include the new parameters.
Fix results data download.

Current result when downloading:

uellue · 2021-08-23T15:47:50Z

We could consider baking a new release soon after this?

sk1p · 2021-08-23T16:10:23Z

Looks good so far! We should include a link or info sign that explains the parameters. We have them documented in https://libertem.github.io/LiberTEM/reference/api.html#libertem.api.Context.create_com_analysis, for example. :-)

👍 - I'll add more information about the parameters. I also thought about always showing the parameter form, as it doesn't take up a lot of space anyway - what do you think?

Current result when downloading: [...]

Thanks, I'll look into that, should be easy to fix.

We could consider baking a new release soon after this?

Yeah, I thought the same - what do you think about including a fix for #633, too, as I'm touching that code anyways?

I noticed that we don't have COM in our Applications section. We could perhaps create a stub for the "phase contrast" application with an API example, link to reference docs and one screenshot for COM, and a link to https://ptychography-4-0.github.io/ptychography/algorithms.html for SSB ptychography?

Sure, I'll add a section to the docs.

uellue · 2021-08-23T16:14:52Z

Looks good so far! We should include a link or info sign that explains the parameters. We have them documented in https://libertem.github.io/LiberTEM/reference/api.html#libertem.api.Context.create_com_analysis, for example. :-)

👍 - I'll add more information about the parameters. I also thought about always showing the parameter form, as it doesn't take up a lot of space anyway - what do you think?

Yes, I was wondering why they were hidden but figured you had your reasons. 😆

We could consider baking a new release soon after this?

Yeah, I thought the same - what do you think about including a fix for #633, too, as I'm touching that code anyways?

Yes, perfect! Would require a bit more change in the CoM API, but should be pretty straightforward, right? :-)

Other points: Thx, great! 👍

sk1p · 2021-08-23T18:46:19Z

/azp run libertem.libertem-data

azure-pipelines · 2021-08-23T18:46:27Z

Azure Pipelines successfully started running 1 pipeline(s).

sk1p · 2021-08-23T20:01:11Z

/azp run libertem.libertem-data

azure-pipelines · 2021-08-23T20:01:20Z

Azure Pipelines successfully started running 1 pipeline(s).

sk1p · 2021-08-24T13:44:32Z

/azp run libertem.libertem-data

azure-pipelines · 2021-08-24T13:44:39Z

Azure Pipelines successfully started running 1 pipeline(s).

sk1p · 2021-08-24T14:13:05Z

@uellue I've pushed the last fixes; if you are happy with this, I can re-build the client and merge - or do you want to push CoM docs also to this PR?

uellue · 2021-08-24T15:54:22Z

or do you want to push CoM docs also to this PR?

If it is OK it could be a quick solution! :-) I can also rebuild the client if I am at it. :-)

sk1p · 2021-08-24T15:55:29Z

If it is OK it could be a quick solution! :-) I can also rebuild the client if I am at it. :-)

Sounds good!

* Include links to API use * Cite seminal paper on CoM * Notebook test coverage for new parameters

uellue · 2021-08-24T18:17:31Z

/azp run libertem.libertem-data

azure-pipelines · 2021-08-24T18:17:39Z

Azure Pipelines successfully started running 1 pipeline(s).

uellue

Looks good, many thanks! :-)

Should be ready to merge now. :-)

sk1p added 3 commits August 23, 2021 15:28

Fix type annotation for ResultFormatRegistry.get_format_by_id

214ebf6

Add .editorconfig

b51dea3

sk1p added enhancement New feature or request GUI labels Aug 23, 2021

sk1p requested a review from uellue August 23, 2021 13:49

sk1p changed the title ~~CoM rotation and flip~~ CoM rotation and flip in the GUI Aug 23, 2021

uellue requested changes Aug 23, 2021

View reviewed changes

.editorconfig Show resolved Hide resolved

src/libertem/analysis/com.py Show resolved Hide resolved

uellue added this to the 0.8 milestone Aug 23, 2021

sk1p added 6 commits August 23, 2021 19:12

Fix result downloading

0aa2c3d

Add a changelog

6f41689

Add detailed description of parameters in info modal

45bd91a

CenterOfMassAnalysis: fix entry of negative angles

da171bf

CoM notebook generator: add flip_y / scan_rotation

c54f87e

WIP: stub of CoM docs in application section

f584aa7

Fix: flip_y and scan_rotation are optional

864d02e

sk1p added 3 commits August 24, 2021 14:06

restrict rotation range to -180 to +180 deg

c6f42ad

Don't immediately re-run after results become available

86cc5f3

Add a test case that covers re-running jobs (and not needing re-runs)

712e5ef

sk1p requested a review from uellue August 24, 2021 14:25

sk1p mentioned this pull request Aug 24, 2021

Annular CoM #1089

Merged

7 tasks

uellue added 2 commits August 24, 2021 20:07

Doc tweaks; test

651845a

* Include links to API use * Cite seminal paper on CoM * Notebook test coverage for new parameters

Rebuilt client

7f54f8a

uellue approved these changes Aug 24, 2021

View reviewed changes

sk1p merged commit 98b1e0f into LiberTEM:master Aug 24, 2021

sk1p deleted the com-rotation-transform branch August 24, 2021 23:15

uellue mentioned this pull request Aug 25, 2021

Possible confusion between current GUI parameters and notebook download #1090

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CoM rotation and flip in the GUI #1087

CoM rotation and flip in the GUI #1087

sk1p commented Aug 23, 2021 •

edited by uellue

Loading

codecov bot commented Aug 23, 2021 •

edited

Loading

uellue left a comment

uellue commented Aug 23, 2021 •

edited by sk1p

Loading

uellue commented Aug 23, 2021

sk1p commented Aug 23, 2021

uellue commented Aug 23, 2021

sk1p commented Aug 23, 2021

azure-pipelines bot commented Aug 23, 2021

sk1p commented Aug 23, 2021

azure-pipelines bot commented Aug 23, 2021

sk1p commented Aug 24, 2021

azure-pipelines bot commented Aug 24, 2021

sk1p commented Aug 24, 2021

uellue commented Aug 24, 2021

sk1p commented Aug 24, 2021

uellue commented Aug 24, 2021

azure-pipelines bot commented Aug 24, 2021

uellue left a comment

CoM rotation and flip in the GUI #1087

CoM rotation and flip in the GUI #1087

Conversation

sk1p commented Aug 23, 2021 • edited by uellue Loading

TODO

Contributor Checklist:

Reviewer Checklist:

codecov bot commented Aug 23, 2021 • edited Loading

Codecov Report

uellue left a comment

Choose a reason for hiding this comment

uellue commented Aug 23, 2021 • edited by sk1p Loading

uellue commented Aug 23, 2021

sk1p commented Aug 23, 2021

uellue commented Aug 23, 2021

sk1p commented Aug 23, 2021

azure-pipelines bot commented Aug 23, 2021

sk1p commented Aug 23, 2021

azure-pipelines bot commented Aug 23, 2021

sk1p commented Aug 24, 2021

azure-pipelines bot commented Aug 24, 2021

sk1p commented Aug 24, 2021

uellue commented Aug 24, 2021

sk1p commented Aug 24, 2021

uellue commented Aug 24, 2021

azure-pipelines bot commented Aug 24, 2021

uellue left a comment

Choose a reason for hiding this comment

sk1p commented Aug 23, 2021 •

edited by uellue

Loading

codecov bot commented Aug 23, 2021 •

edited

Loading

uellue commented Aug 23, 2021 •

edited by sk1p

Loading