OpenHKL issueshttps://jugit.fz-juelich.de/mlz/openhkl/-/issues2022-09-26T11:14:47+02:00https://jugit.fz-juelich.de/mlz/openhkl/-/issues/543Core: Pixel statistics do not respect masks2022-09-26T11:14:47+02:00Raza, ZamaanCore: Pixel statistics do not respect masksMasked pixels should be excluded from histograms.Masked pixels should be excluded from histograms.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/539core: Peaks generated by Predictor are stored on the heap, possible memory leak2022-09-14T15:36:30+02:00Raza, Zamaancore: Peaks generated by Predictor are stored on the heap, possible memory leakFrom `Predictor.cpp`
```
std::vector<Peak3D*> peaks;
for (const auto& [hkl, event] : events) {
Peak3D* peak(new Peak3D(data, hkl)); // here
Eigen::Vector3d center = {event.px, event.py, event.frame};
// s...From `Predictor.cpp`
```
std::vector<Peak3D*> peaks;
for (const auto& [hkl, event] : events) {
Peak3D* peak(new Peak3D(data, hkl)); // here
Eigen::Vector3d center = {event.px, event.py, event.frame};
// setShape may disable the peak if the centre is out of bounds
peak->setShape(Ellipsoid(center, 1.0));
if (peak->selected()) {
peak->setUnitCell(unit_cell);
peaks.push_back(peak);
}
}
```
Prefer smart pointers instead, see `PeakFinder` for implementation.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/533swap storage order in HDF5 matrices2022-08-11T17:56:19+02:00Wuttke, Joachimj.wuttke@fz-juelich.deswap storage order in HDF5 matricesto avoid costly copies.
See MR !423, which unfortunately has merge conflicts.to avoid costly copies.
See MR !423, which unfortunately has merge conflicts.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/527Core: batch unit cells should have different names based on their data set2022-08-09T09:10:38+02:00Raza, ZamaanCore: batch unit cells should have different names based on their data setFor a data reduction with multiple data sets, it should be possible to distinguish the unit cells based on which data set they belong to.For a data reduction with multiple data sets, it should be possible to distinguish the unit cells based on which data set they belong to.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/521DataSet::dataAt is unused, and ultraslow when used in loop2022-08-03T10:15:23+02:00Wuttke, Joachimj.wuttke@fz-juelich.deDataSet::dataAt is unused, and ultraslow when used in loopFunction DataSet::dataAt is currently unused. Should however a developer use it in a loop, he will be badly surprised by how incredibly slow it is: for every single data point, it will re-read the entire raw file.
Therefore its is best ...Function DataSet::dataAt is currently unused. Should however a developer use it in a loop, he will be badly surprised by how incredibly slow it is: for every single data point, it will re-read the entire raw file.
Therefore its is best to remove this function for good.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/511unify handling of diffractometer in InstrumentState2022-08-01T10:00:54+02:00Wuttke, Joachimj.wuttke@fz-juelich.deunify handling of diffractometer in InstrumentStateClass `InstrumentState` has a member `_diffractometer` but function `InstrumentState::state` takes an argument `diffractometer`.Class `InstrumentState` has a member `_diffractometer` but function `InstrumentState::state` takes an argument `diffractometer`.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/501Minimizer: rm unused member function, make getter functions const, correct typos2022-07-29T12:36:31+02:00Wuttke, Joachimj.wuttke@fz-juelich.deMinimizer: rm unused member function, make getter functions const, correct typosSeveral getter functions in classes Refiner and RefinementBatch are blocked from being made `const` by member function `RefinementBatch::_cost_function`, which is unused and should be removed.
See pending solution in !504.Several getter functions in classes Refiner and RefinementBatch are blocked from being made `const` by member function `RefinementBatch::_cost_function`, which is unused and should be removed.
See pending solution in !504.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/482Core: outlier rejection methods from Scikit-learn2022-07-25T15:03:09+02:00Raza, ZamaanCore: outlier rejection methods from Scikit-learnThe follwoing methods are robust, community tested and open source, so should be easy enough to implement. See also #138, which is inspired by a closed source and undocumented algorithm from another code.
https://scikit-learn.org/stable...The follwoing methods are robust, community tested and open source, so should be easy enough to implement. See also #138, which is inspired by a closed source and undocumented algorithm from another code.
https://scikit-learn.org/stable/modules/outlier_detection.htmlRaza, ZamaanRaza, Zamaanhttps://jugit.fz-juelich.de/mlz/openhkl/-/issues/439Core: generating shapes from *predicted* peaks2022-07-06T09:31:34+02:00Raza, ZamaanCore: generating shapes from *predicted* peaksIn order to generate a better shape model, we might try to "bootstrap" the shape model by:
1. constructing a shape model from *found* peaks
2. applying that model to predicted peaks
3. constructing a shape model from the resulting *predi...In order to generate a better shape model, we might try to "bootstrap" the shape model by:
1. constructing a shape model from *found* peaks
2. applying that model to predicted peaks
3. constructing a shape model from the resulting *predicted* peaks
4. applying that model to the same predicted peaks
If we do not integrate the predicted peaks after step 2, this will fail, and we end up with a shape model with 0 peaks. This is not the intended behaviour.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/425Core: bad OpenMP parallelisation of integration algorithm2022-05-13T08:11:01+02:00Raza, ZamaanCore: bad OpenMP parallelisation of integration algorithmAt first glance, `IPeakIntegrator::integrate` seems fairly trivial to OpenMP parallelise; however, in retrospect it seems that the results it generates are inconsistent with serial integration results. This needs to be reexamined.At first glance, `IPeakIntegrator::integrate` seems fairly trivial to OpenMP parallelise; however, in retrospect it seems that the results it generates are inconsistent with serial integration results. This needs to be reexamined.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/377Testing: Add ShapeCollection unit test2022-02-09T11:34:02+01:00Raza, ZamaanTesting: Add ShapeCollection unit testA core algorithm that is not included in unit testing.A core algorithm that is not included in unit testing.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/372Core: better shape prediction algorithm2022-02-08T12:07:01+01:00Raza, ZamaanCore: better shape prediction algorithmVariation in spot shape is purely an effect of detector geometry. This is why for a cylindrical detector like BioDiff, the spots at the top and bottom of the detector image are the most distorted and difficult to integrate. In reciprocal...Variation in spot shape is purely an effect of detector geometry. This is why for a cylindrical detector like BioDiff, the spots at the top and bottom of the detector image are the most distorted and difficult to integrate. In reciprocal space, the reflection shapes are *independent of position*.
Therefore, there are a couple of possibilities that would allow us to average over *all* strong peaks to get a guess for the shape, rather than over *local* peaks.
1. Transform the detector image from a cylinder to a sphere, then integrate as usual.
2. Transform the detector spots to 3D reciprocal space ellipsoids. Compute a mean ellipsoid, then transform the resulting shape back to a detector position-dependent ellipse.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/371Core: better integration at high resolution2022-01-13T12:16:26+01:00Raza, ZamaanCore: better integration at high resolutionInadequate merge statistics seem to be a result of poor integration of high resolution peaks, namely due to the low quality of the *shape* assigned to the high resolution peaks. The shape assignment procedure assumes that strong peaks su...Inadequate merge statistics seem to be a result of poor integration of high resolution peaks, namely due to the low quality of the *shape* assigned to the high resolution peaks. The shape assignment procedure assumes that strong peaks surrounding a high resolution peak are well characterised enough to give a good estimate for the shape of the peak via the mean covariance. This is often not the case because 1. there are often few neighbours of high resolution peaks; 2. the neighbours of high resolution peaks often also have poorly characterised shapes.
The solution to this could come in the form of a "bootstrapping", first using the found peaks to generate an initial guess for the shapes of the predicted peaks, but then using these intiaal guesses to iteratively refine the shapes at high resolution.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/367Core: shape determination of high-resolution peaks is bad2021-12-09T10:03:37+01:00Raza, ZamaanCore: shape determination of high-resolution peaks is badFrom a *simulated* data set, the instrument states (detector position, sample position and orientation and incident wavevector) are known, and perfectly defined, so the only thing that needs refining is the unit cell, removing many unkno...From a *simulated* data set, the instrument states (detector position, sample position and orientation and incident wavevector) are known, and perfectly defined, so the only thing that needs refining is the unit cell, removing many unknowns. Here is a high resolution peak `(11, 7, 14)`:
![hi-res](/uploads/c6e1adc86abb705662a2c3a40da67463/hi-res.png)
vs a low resolution peak `(1, -2, 11)`:
![low-res](/uploads/c0899c2f51d4fe2c49dd711594cd2aad/low-res.png)
The issue seems to be in the way the shape is determined. The high resolution peaks lack sufficient neighbouring strong peaks to accurately determine the shape, and since they tend to have a large angular width (mosaicity), they intersect many frames and the "precession" across the image is not captured.
One possible method of resolving this is "bootstrapping" to determine peak shapes. We first use the peaks from the image analysis step to build an initial `ShapeCollection`, and use these shapes to integrate the predicted peaks. Then we build a new `ShapeCollection` from the *predicted* peaks, in the hope that we have more good shapes in the high resolution regime.Raza, ZamaanRaza, Zamaanhttps://jugit.fz-juelich.de/mlz/openhkl/-/issues/363Core: rejection flag for fullies2021-11-24T09:57:10+01:00Raza, ZamaanCore: rejection flag for fulliesIt would be useful to label peaks that only appear on one frame as fullies.It would be useful to label peaks that only appear on one frame as fullies.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/356Core: loading raw .tiff files direct from BioDiff2022-08-08T09:36:04+02:00Raza, ZamaanCore: loading raw .tiff files direct from BioDiffCurrently an additional image processing step (usually performed via ImageJ) is required to convert `.tiff` files from BioDiff into something readable by NSXTool. It would help the user to be able to read these files directly.Currently an additional image processing step (usually performed via ImageJ) is required to convert `.tiff` files from BioDiff into something readable by NSXTool. It would help the user to be able to read these files directly.Christian TrageserChristian Trageserhttps://jugit.fz-juelich.de/mlz/openhkl/-/issues/354Core: parallelisation via threading2021-11-09T09:40:55+01:00Raza, ZamaanCore: parallelisation via threadingOpenMP is not available via package managers under MacOS, and could potentially be problematic for users to install. A useful alternative would be to implement threading, which is platform-independent, as alternative that can be specifie...OpenMP is not available via package managers under MacOS, and could potentially be problematic for users to install. A useful alternative would be to implement threading, which is platform-independent, as alternative that can be specified via cmake.https://jugit.fz-juelich.de/mlz/openhkl/-/issues/331Core: detector position is never refined2022-07-30T15:23:34+02:00Raza, ZamaanCore: detector position is never refinedWhenever refinement is done, the deltas for the detector offset are always zeroWhenever refinement is done, the deltas for the detector offset are always zeroRaza, ZamaanRaza, Zamaanhttps://jugit.fz-juelich.de/mlz/openhkl/-/issues/319Avoid exposing move semantics to the external Python API to preclude undefine...2021-07-26T11:51:30+02:00Ammar NejatiAvoid exposing move semantics to the external Python API to preclude undefined behaviourConsider a C++ class `DataSet` with the following constructor:
```
DataSet::DataSet(std::shared_ptr<IDataReader> reader)
: _reader{std::move(reader)}
{
// do something...
}
```
Exposing this constructor to the Python API leads to ...Consider a C++ class `DataSet` with the following constructor:
```
DataSet::DataSet(std::shared_ptr<IDataReader> reader)
: _reader{std::move(reader)}
{
// do something...
}
```
Exposing this constructor to the Python API leads to unknown/undefined behaviour:
```
import pynsx as nsx
expr = nsx.Experiment('test', 'BioDiff2500')
diffractometer = expr.diffractometer()
reader = nsx.HDF5DataReader("datafile.nsx", diffractometer)
data = nsx.DataSet(reader)
reader.someMethod() # unknown behaviour since `reader` is moved!
reader.someVariable # unknown result since `reader` is moved!
```
NOTE: cppreference for [std::move](https://en.cppreference.com/w/cpp/utility/move) states that
> Unless otherwise specified, all standard library objects that have been moved from are placed in a "valid but unspecified state".https://jugit.fz-juelich.de/mlz/openhkl/-/issues/315Core: how does GaussianIntegrator work?2021-07-18T11:18:11+02:00Raza, ZamaanCore: how does GaussianIntegrator work?There seems to be no documentation or references for `GaussianIntegrator`, yet it is the most computationally heavy of the integrators.There seems to be no documentation or references for `GaussianIntegrator`, yet it is the most computationally heavy of the integrators.