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

Documentation for excitation and emission code + a bug fix. #59

Merged
merged 3 commits into from
Jul 1, 2024

Conversation

ashesh-0
Copy link
Collaborator

This also fixes a bug in cross section computation, where one needs to multiply by 1000. however, pint already does that when converting to approapriate units and therefore there is no need to explicitly multiply 1000.

@ashesh-0 ashesh-0 requested a review from tlambert03 June 30, 2024 10:54
Copy link

codecov bot commented Jun 30, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.81%. Comparing base (2f7d9c9) to head (65b21c7).

Files Patch % Lines
src/microsim/schema/_emission.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main      #59   +/-   ##
=======================================
  Coverage   85.81%   85.81%           
=======================================
  Files          49       49           
  Lines        2792     2792           
=======================================
  Hits         2396     2396           
  Misses        396      396           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tlambert03
Copy link
Owner

thanks @ashesh-0!

by the way, in thinking about @federico-carrara's application, I think we need to add (and internally use) an additional output of the simulation. I think we should break out some of the logic in emission_flux into an irradiance(self)->xr.DataArray method. namely: both ground_truth and irradiance are meaningful (and unrelated) "ground truth"-like objects.

irradiance should be a 5D array (W, C, Z, Y, X) where the values have units of Watts/cm2, where W is wavelength, and C represents something about the optical config. For example, in the concrete case of spectral detection on a confocal scope, there would really only be a single C, since there is usually only a single excitation setup on those types of scopes. It may well have multiple lasers simultaneously on, and that would be reflected in little peaks of intensity in the W channel.

The binning used on W is obviously important, and I think, after further reflection on the spectral detection use case, that we should normalize bin edges in such a way as to be useful for all of the different excitation/emission things we need to consider.

I know that's still quite vague. Perhaps the three of us could zoom sometime and I can explain in person?

@tlambert03 tlambert03 merged commit 3213a79 into tlambert03:main Jul 1, 2024
16 of 18 checks passed
@federico-carrara
Copy link
Contributor

Hi Talley!
It would be great to discuss this thing together. Also considering that I went on doing some experiments for obtaining the "optical image" in the spectral setting and I needed to modify parts of the existing code. Therefore, it would be helpful to discuss to avoid having extremely divergent versions!

@tlambert03
Copy link
Owner

Therefore, it would be helpful to discuss to avoid having extremely divergent versions!

sounds good. in general, we should all treat main as the version. so it's all of our responsibility not to "rely" to much on anything that is too divergent from main. If you make significant changes that you find useful, try to make a PR to get them into main, and as main changes, make sure to continually merge main into your experimental branches.

@ashesh-0
Copy link
Collaborator Author

ashesh-0 commented Jul 1, 2024

Hi @tlambert03 , makes sense. This week, I'm available for a call on all days 9:00AM EDT to 2pm EDT except on Friday. On Friday, from 10AM to 2pm EDT.

@federico-carrara
Copy link
Contributor

I guess it would make sense for @ashesh-0 and me to call you when we are in the office together. So, I propose either Wednesday from 9 AM to 1 PM or Friday from 10 AM to 1PM (Boston time). In case this doesn't work for you, we can certainly find different slots :) Thank you!

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.

3 participants