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

Use mcx SLIT source for MSOTAcuity and MSOTInvision sources #266

Conversation

lkeegan
Copy link
Contributor

@lkeegan lkeegan commented Mar 8, 2024

Please check the following before creating the pull request (PR):

  • Did you run automatic tests?
  • Did you run manual tests?
  • Is the code provided in the PR still backwards compatible to previous SIMPA versions?

List any specific code review questions

List any special testing requirements

Requires mcx version >= v2024.2

Additional context (e.g. papers, documentation, blog posts, ...)

Provide issue / feature request fixed by this PR

  • MSOTAcuityIlluminationGeometry
    • Change in source due to fixing of sign error
  • MSOTInVisionIlluminationGeometry
    • Change in source due to use of gaussian broadening instead of step function
    • Now only requires a single mcx run using the new multiple sources feature
  • Note: requires mcx version >= v2024.2

lkeegan added 2 commits March 8, 2024 11:30
- MSOTAcuityIlluminationGeometry
  - Change in source due to fixing of sign error
- MSOTInVisionIlluminationGeometry
  - Change in source due to use of gaussian broadening instead of step function
  - Now only requires a single mcx run using the new multiple sources feature
- Note: requires mcx version >= v2024.2
@lkeegan
Copy link
Contributor Author

lkeegan commented Mar 8, 2024

@kdreher this works for me with mcx compiled from this branch: fangq/mcx#214

@lkeegan lkeegan marked this pull request as ready for review March 11, 2024 09:26
@lkeegan
Copy link
Contributor Author

lkeegan commented Mar 11, 2024

@kdreher this should now work with the latest nightly build of mcx

Copy link
Collaborator

@kdreher kdreher left a comment

Choose a reason for hiding this comment

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

The changes look good to me.
I think we'll need a couple more changes before we merge this, though:

  • When I try to run the minimal_optical_simulation.py with SAVE_REFLECTANCE and SAVE_PHOTON_DIRECTION as True, I get a shape mismatch error. I already tried the change suggested in this pull request but that didn't work. So I guess there is another issue.
  • We need to adjust the invision example
  • We need to adjust the README.md to the new way of using mcx

@kdreher kdreher merged commit fdd19f6 into IMSY-DKFZ:main Mar 13, 2024
12 checks passed
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