Implement detailed balance settings#162
Conversation
henrikjacobsenfys
added
[scope] enhancement
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #162 +/- ##
===========================================
+ Coverage 97.93% 98.02% +0.09%
===========================================
Files 43 45 +2
Lines 2715 2790 +75
Branches 472 484 +12
===========================================
+ Hits 2659 2735 +76
Misses 33 33
+ Partials 23 22 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
damskii9992
left a comment
damskii9992
left a comment
There was a problem hiding this comment.
+1 for standardizing to "normalize" instead of "divide by temperature" and use it consistently :P
Looks good to me, just a few small things and a potential improvement / learning opportunity.
| self, | ||
| use_detailed_balance: bool = True, | ||
| normalize_detailed_balance: bool = True, | ||
| display_name: str = 'MySampleModel', |
There was a problem hiding this comment.
You probably don't want this default display name :P
There was a problem hiding this comment.
I definitely did not copy/paste anything. Nope, no copy paste at all.
| display_name : str, default='MySampleModel' | ||
| Display name of the model. | ||
| unique_name : str | None, default=None | ||
| Unique name of the model. If None, a unique name will be generated. |
| from easydynamics.base_classes.easydynamics_base import EasyDynamicsBase | ||
|
|
||
|
|
||
| class DetailedBalanceSettings(EasyDynamicsBase): |
There was a problem hiding this comment.
This could probably be a dataclass. Seems ideal for it, to avoid writing boiler-plate code :)
|
I played with running some scripts I had before and one is not giving me the same results. As expected or something more sinister? import numpy as np
import scipp as sc
from easydynamics.analysis.analysis1d import Analysis1d
from easydynamics.experiment import Experiment
from easydynamics.sample_model import InstrumentModel, SampleModel
from easydynamics.sample_model.components.gaussian import Gaussian
Q = sc.array(dims=['Q'], values=[1.0], unit='1/Angstrom')
energy = sc.array(dims=['energy'], values=np.linspace(-1.0, 1.0, 5), unit='meV')
data = sc.array(
dims=['Q', 'energy'],
values=[[1.0, 1.0, 1.0, 1.0, 1.0]],
variances=[[1.0, 1.0, 1.0, 1.0, 1.0]],
)
experiment = Experiment(data=sc.DataArray(data=data, coords={'Q': Q, 'energy': energy}))
sample_model = SampleModel(
components=Gaussian(area=1.0, center=0.0, width=0.2),
temperature=100.0,
)
instrument_model = InstrumentModel() # empty resolution model
analysis = Analysis1d(
experiment=experiment,
sample_model=sample_model,
instrument_model=instrument_model,
Q_index=0,
)
print("About to call analysis.calculate()")
print(analysis.calculate())
The current branch fails with |
(I don't know why, but in this branch AnalysisBase inherited directly from the easyscience base classes instead of my own. Should be fixed now. Also, I'm expecting the results to be different now, with calculate giving a higher value at positive energy than negative energy because of detailed balancing.) |
|
I decided to make a settings folder, since I'll some time soon^TM introduce fit settings as well, and it seems nicer to have all settings in one place - perhaps even a settings class? (or dataclass). I might also one day do default plot settings or other types of settings. |
3 participants
Fixes #160
No new tests yet. Will need to test that
Analysisuses detailed balancing when convolution is not used.