fix: theoretical muD estimation #189
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #189 +/- ##
=======================================
Coverage 99.31% 99.31%
=======================================
Files 5 5
Lines 292 292
=======================================
Hits 290 290
Misses 2 2
🚀 New features to boost your workflow:
|
No this is separate from getmuD. I think we also allow users to compute muD and apply corrections at the same step (e.g. using |
| # Option 4: Using packing fraction | ||
| labpdfproc zro2_mo.xy -p ZrO2,17.45,0.2 | ||
| # Option 3: Theoretical estimation | ||
| labpdfproc zro2_mo.xy -t ZrO2,17.45,1.2,1.0 |
There was a problem hiding this comment.
(Just realized that I forgot to write comments for where I changed, sorry) Here I removed the packing fraction option example
| # Option 4: Using packing fraction | ||
| args = Namespace(theoretical_from_packing="ZrO2,17.45,0.3") | ||
| # Option 3: Theoretical estimation | ||
| args = Namespace(theoretical_estimation="ZrO2,17.45,1.2,1.0") |
There was a problem hiding this comment.
Remove packing fraction
| "Specify the chemical formula, incident x-ray energy (in keV), " | ||
| "and packing fraction (0 to 1), " + theoretical_mud_hmsg_suffix | ||
| "sample mass density (in g/cm^3), " | ||
| "and capillary diameter (in mm) " |
There was a problem hiding this comment.
add a requirement for specifying capillary diameter
| return sample_composition, energy, mass_density_or_packing_fraction | ||
| sample_mass_density = float(parts[2]) | ||
| diameter = float(parts[3]) | ||
| return sample_composition, energy, sample_mass_density, diameter |
There was a problem hiding this comment.
Again add capillary diameter
| args.energy, | ||
| sample_mass_density=args.sample_mass_density, | ||
| ) | ||
| * args.diameter |
There was a problem hiding this comment.
muD = mu * D
|
I can merge this if you want (it is conflicted so that needs to be fixed) but my concern is that the CLI is not the way we want it so we can't release this way. Do you want to fix things on this PR or another one? |
I can resolve the conflicts here so we can merge this PR, and address the CLI issue in a separate PR. Do you prefer to keep the different muD computation methods within getmuD only (as we discussed in #181)? I think it is also helpful to give users the option to compute muD during the correction. |
|
@sbillinge ready for review. |
yes, I am working off the understanding that we are being guided by the discussion in #181. However, if we have this structure for |
|
Honestly I am inclined to just close this issue as it is a bunch of small fixes to a CLI that we are about to rip up and throw away. there may be a few lines of code that we can save, but when we make the new CLE I think how we handle the functions will change a bit too so I am not sure it makes any sense to keep any of this? |
Got it. Yeah I like this idea - I created a new issue (#191) for this because I feel like this is a separate problem from the |
closes #188, closes #187
@sbillinge ready for review. I've fixed a few issues related to the theoretical muD estimation and this should be the last issue before you check the docs in more detail. I'll do the recookiecut next.