-
Notifications
You must be signed in to change notification settings - Fork 3
Update to scipp-0.8 #164
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
Update to scipp-0.8 #164
Changes from all commits
d1762fd
56b5348
adeb7b0
30ed490
e10db0d
f70b317
8da54b3
385a390
edd1c5b
d42e62e
1d67334
8e09d3b
70434b2
3169eb2
a88c377
301ff26
a717d3b
1f130c4
63e5959
d18bac7
e845604
db1ecbf
f90831e
861a4b9
0015565
1e10c74
f55d592
8e5fbfc
9b3f4e1
48c8588
3a6cdb0
fc917ba
39acd1d
4ac087c
a446653
c235e4c
f69a7ea
5bff6fb
1331817
9fd2246
4580811
7afd8fa
2125da9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,4 +2,4 @@ python: | |
| - 3.7 | ||
|
|
||
| scipp: | ||
| - 0.7 | ||
| - 0.8 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ source: | |
| requirements: | ||
| build: | ||
| - cmake | ||
| - gxx_linux-64 9.3.* [linux64] | ||
| - gxx_linux-64 11.1.* [linux64] | ||
| - git | ||
| - ninja | ||
| - python {{ python }} | ||
|
|
@@ -26,6 +26,7 @@ test: | |
| - neutron | ||
| requires: | ||
| - h5py | ||
| - ipykernel = 6.3.1 # see https://github.com/ipython/ipykernel/issues/771 | ||
| - ipywidgets | ||
| - matplotlib-base | ||
| - psutil | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably need to pin |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ channels: | |
| dependencies: | ||
| - ess-dmsc::ess-streaming-data-types | ||
| - h5py | ||
| - ipykernel = 6.3.1 # see https://github.com/ipython/ipykernel/issues/771 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually don't think these files (in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the contrary, this is what users will use to install. See https://scipp.github.io/scippneutron/getting-started/installation.html#with-the-provided-environment-file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, that's what they are used for! Thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But once again we risk those being out of sync with https://github.com/scipp/scippneutron/blob/main/scippneutron-developer.yml and https://github.com/scipp/scippneutron/blob/main/scippneutron-developer-no-mantid.yml There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And these won't work for windows, etc... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Not really, because the purpose is different: The user-envs do not include dev dependencies. And valid versions for actual dependencies should be setup correctly in |
||
| - ipympl | ||
| - ipython | ||
| - ipywidgets | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -131,7 +131,7 @@ | |
| "source": [ | ||
| "sample.coords['two_theta'] = scn.two_theta(sample)\n", | ||
| "vanadium.coords['two_theta'] = scn.two_theta(vanadium)\n", | ||
| "two_theta_bins = sc.Variable(['two_theta'],\n", | ||
| "two_theta_bins = sc.Variable(dims=['two_theta'],\n", | ||
| " unit=sc.units.rad,\n", | ||
| " values=np.linspace(0.0, np.pi, num=2000))" | ||
| ] | ||
|
|
@@ -266,6 +266,23 @@ | |
| "mon" | ||
| ] | ||
| }, | ||
| { | ||
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "We intend to normalize each event to the relative monitor counts (compared to the total monitor counts).\n", | ||
| "We use `sum` to compute the total monitor counts and obtain the relative counts using division:" | ||
| ] | ||
| }, | ||
| { | ||
| "cell_type": "code", | ||
| "execution_count": null, | ||
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": [ | ||
| "mon /= mon.sum('wavelength')" | ||
| ] | ||
| }, | ||
| { | ||
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
|
|
@@ -331,12 +348,9 @@ | |
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": [ | ||
| "dspacing_bins = sc.Variable(\n", | ||
| " ['dspacing'],\n", | ||
| " values=np.arange(0.3, 2.0, 0.001),\n", | ||
| " unit=sc.units.angstrom)\n", | ||
| "hist = sc.Dataset({'sample':sc.histogram(dspacing_sample, dspacing_bins),\n", | ||
| " 'vanadium':sc.histogram(dspacing_vanadium, dspacing_bins)})\n", | ||
| "dspacing_bins = sc.arange(dim='dspacing', start=0.3, stop=2.0, step=0.001, unit=sc.units.angstrom)\n", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I mention this because someone had posted a link to position-only arguments at some point and it sounded like there was interest in them, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. We need to decide how to handle those in our |
||
| "hist = sc.Dataset(data={'sample':sc.histogram(dspacing_sample, bins=dspacing_bins),\n", | ||
| " 'vanadium':sc.histogram(dspacing_vanadium, bins=dspacing_bins)})\n", | ||
| "sc.show(hist['spectrum', 0:3]['dspacing', 0:7])" | ||
| ] | ||
| }, | ||
|
|
@@ -396,15 +410,13 @@ | |
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": [ | ||
| "two_theta = sc.Variable(['two_theta'],\n", | ||
| " unit=sc.units.rad,\n", | ||
| " values=np.linspace(0.0, np.pi, num=16))\n", | ||
| "two_theta = sc.linspace(dim='two_theta', unit='rad', start=0.0, stop=np.pi, num=16)\n", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
But we should also keep in mind that scipp requires an extra |
||
| "\n", | ||
| "focussed_sample = sc.groupby(dspacing_sample, 'two_theta', bins=two_theta).bins.concatenate('spectrum')\n", | ||
| "focussed_vanadium = sc.groupby(dspacing_vanadium, 'two_theta', bins=two_theta).bins.concatenate('spectrum')\n", | ||
| "norm = sc.histogram(focussed_vanadium, dspacing_bins)\n", | ||
| "norm = sc.histogram(focussed_vanadium, bins=dspacing_bins)\n", | ||
| "focussed_sample.bins /= sc.lookup(func=norm, dim='dspacing')\n", | ||
| "normalized = sc.histogram(focussed_sample, dspacing_bins)" | ||
| "normalized = sc.histogram(focussed_sample, bins=dspacing_bins)" | ||
| ] | ||
| }, | ||
| { | ||
|
|
@@ -420,7 +432,7 @@ | |
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": [ | ||
| "sc.plot(normalized, vmin=0, vmax=2)" | ||
| "sc.plot(normalized, vmin=sc.scalar(0), vmax=sc.scalar(0.4))" | ||
| ] | ||
| }, | ||
| { | ||
|
|
@@ -447,7 +459,7 @@ | |
| ], | ||
| "metadata": { | ||
| "kernelspec": { | ||
| "display_name": "Python 3", | ||
| "display_name": "Python 3 (ipykernel)", | ||
| "language": "python", | ||
| "name": "python3" | ||
| }, | ||
|
|
@@ -461,7 +473,7 @@ | |
| "name": "python", | ||
| "nbconvert_exporter": "python", | ||
| "pygments_lexer": "ipython3", | ||
| "version": "3.7.10" | ||
| "version": "3.8.10" | ||
| } | ||
| }, | ||
| "nbformat": 4, | ||
|
|
||
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.
Having all these different env files is becoming a nightmare to maintain. Is there really no alternative to having a new file which is 98% identical to the other env files? Aren't there tags you can add in env file if you want something install only on certain platforms? I thought we did this with some of the data streaming packages which don't install on windows.
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.
See in the conda/meta.yaml file
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.
Couldn't agree more. Added as item in scipp/scipp#2163.
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.
I mainly made this change to ensure that we could complete the CI, i.e. scippneutron worked with 0.8. These only work with the recipe files as far as I can determine. https://conda-devenv.readthedocs.io/en/latest/usage.html is the only other solution i am aware of.
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.
See note I just added in scipp/scipp#2163 (comment), might
conda debugbe a solution for creating the envs frommeta.yml?Uh oh!
There was an error while loading. Please reload this page.
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.
It could work, but seems that you need to source the environment script from within a conda working directory, that is to say, it doesn't look like the most convenient thing to use as a developer, but I do like the fact that this might result in us being able to discard our environment files all-together.
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.
Yes, you need to have a base env with (I presume)
conda-buildinstalled. But then something like this may work:I suppose there could be a helper script shipped with the repo that does this.