-
Notifications
You must be signed in to change notification settings - Fork 14
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
add an adjustable intensity factor for XPS spectra #642
add an adjustable intensity factor for XPS spectra #642
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #642 +/- ##
==========================================
- Coverage 75.85% 75.77% -0.09%
==========================================
Files 60 60
Lines 4312 4317 +5
==========================================
Hits 3271 3271
- Misses 1041 1046 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @superstar54. Though I agree that this is a necessary feature, I think that the code should scale the intensity of the experimental data rather than the simulated data.
This is for the simple reason that, as the intensities of the computed spectra are modified whenever intensity
is changed, the Result
panel has to re-draw the entire plot from scratch. Scaling the intensity of the experimental data instead means that only one trace
in the plot object needs to be updated, which is considerably faster.
I have a potential way to do this, but the changes are too extensive to easily show as suggested changes, so I will open a PR to your feature branch shortly.
As discussed with @PNOGillespie , we think it's better to provide an initial scale value for the user. |
@PNOGillespie Now, the app will calculate an initial guess for the intensity factor, by aligning the max value of the total spectra with the max value of the experimental data. Please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! I think the last change particularly does a lot of good for usability. Good work @superstar54.
As suggested by @PNOGillespie in this comment. fix #627
Now, the user can scale the calculated spectra and compare them with experimental data. Here is an example for the PA molecule, green color is the experimental data.