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

recOrder buttons working via CLI #347

Merged
merged 8 commits into from
Apr 25, 2023
Merged

recOrder buttons working via CLI #347

merged 8 commits into from
Apr 25, 2023

Conversation

talonchandler
Copy link
Collaborator

@talonchandler talonchandler commented Apr 24, 2023

@ziw-liu @mattersoflight I can demo my progress tomorrow to help us put together a plan.

I've tested this branch against waveorder's pol-refactor branch.

Confirmed working:

  • torch installation on hummingbird
  • new CLI calculate-transfer-function and apply-inverse-transfer-function can perform all reconstruction steps
  • GUI buttons are connected to the new CLI: all 3 acquisition buttons (in both 2D and 3D modes) return results to the screen. Out-of-the-box polarization acquisitions look good. The GUI calls the CLI functions and spits out commands to recreate the transfer functions and reconstructions...initial tests look good.

Major limitations to address:

  • phase reconstructions need some tuning...I expect I've introduced a bug on the recOrder side based on my tests, but I'll look closer tomorrow
  • background corrections are not connected...I had some issues handling strings vs. paths in the yaml file. @ziw-liu I may grab your help here.
  • regularization parameters are not connected and currently use defaults...I had an issue handling a variable number of parameters and passing to kwargs. @ziw-liu if you have capacity I'll grab you here as well.
  • I have implemented a very simple file organization scheme (all files go into the snap_directory, and I'm currently recalculating the transfer functions for every acquisition. A better location for these files will enable easy reuse.

Minor limitations:

  • haven't implemented an end-to-end reconstruct method yet...this will be a fairly easy last step
  • haven't removed much of the unused code in recOrder...commented for now to smooth the transition
  • gpu-related buttons are no longer connected
  • although I'm saving the transfer functions to file, I don't have easy visualizations working because of minor issues related to real vs. complex values.
  • docs, CLI automated testing, much more!
  • apply_inverse_transfer_function is where most of the densest recOrder-waveorder logic lives (these recOrder settings require these TFs applied in this way to these waveorder models). I could likely use some improvement to my use of iohub.

@talonchandler talonchandler marked this pull request as ready for review April 25, 2023 02:28
@talonchandler
Copy link
Collaborator Author

Merging into 0.4.0dev. Will write TODOs into issues.

@talonchandler talonchandler merged commit f3a90db into 0.4.0dev Apr 25, 2023
@talonchandler talonchandler deleted the cli-dev branch April 25, 2023 02:28

class _BirefringenceApplyInverseSettings(BaseModel):
background_path: str = (
None # I'd DirectoryPath but it's not JSON serializable?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need the following :

from pydantic import BaseModel, validator, DirectoryPath

class _BirefringenceApplyInverseSettings(BaseModel):
background_path: str = ''
    @validator('background_path')
    def check_dir_path(cls, v):
        return DirectoryPath().validate(v)

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