Ticket #11573 (closed: fixed)

Opened 5 years ago

Last modified 5 years ago

IntegrateMDHistoWorkspace

Reported by: Owen Arnold Owned by: Owen Arnold
Priority: critical Milestone: Release 3.4
Component: Diffraction Keywords: vates
Cc: Blocked By:
Blocking: Tester: Federico Montesino Pouzols

Description

This is a requirement for #11572. See that ticket for some of the use cases.

This is somewhat similar to http://docs.mantidproject.org/nightly/algorithms/Integration-v1.html except for that we IncludePartialBins as ON by default.

Note that input workspaces should be IMDHistoWorkspaces. There are two reasons for this.

1) It makes processing a lot simpler. 2) For the use-cases in the aforementioned ticket, which demonstrates this, if the input to CutMD is an MDEventWorkspace, BinMD can do the whole job in one step, so this functionality already exists and is properly handled.

Change History

comment:1 Changed 5 years ago by Owen Arnold

  • Status changed from new to inprogress

refs #11573. Enhance MDBoxImplicitFunction

We need to be able to determine the fractional overlap between a rectilinear box represented by a set of vertexes and the box implicit function. This is a general feature.

I've added quite a few functional tests for this, but have not done anything about performance yet.

Changeset: 97fa168083106c2053ab48f0771dceb18a55838a

comment:2 Changed 5 years ago by Owen Arnold

refs #11573. One failing test.

New algorithm added Some tests implemented MDBoxImplicitFunction updated and tested MDIterator updated to give bounding box extents

Last change should be key to fixing current failing test.

Changeset: cd8c18f10ca81ac379dc767085fda5b9d2d5de6d

comment:3 Changed 5 years ago by Owen Arnold

refs #11573. Fix failing test.

Fix the failing test by using the extents rather than trying to calculate extents from vertexes. This should be faster too.

Added a performance test. On my machine this is running in approx 5.5 seconds.

Changeset: 9cfd2bd82737db2f1ed3e22ba08e82ef3ed3c719

comment:4 Changed 5 years ago by Owen Arnold

refs #11573. Reduce from O(N2) to more like a O(N) problem

Reduce from O(N2) problem to something way more like a O(N) problem. Results in large speed improvement. Have not yet used OMP. That will be next.

Changeset: cdc7744992764ff2d59f289daee86a05c5d8fa33

comment:5 Changed 5 years ago by Owen Arnold

refs #11573. OMP loop over outer iterator

Add progress reporting

Add omp loop over the output structure of iterators. This is actually a nice parallel problem.

Changeset: eb322badad4a3e9cbbe1b9743165169f42a2c494

comment:6 Changed 5 years ago by Owen Arnold

refs #11573. Docs and doctest

Changeset: 2cee28130b8d7df40d3c85423b0907fe293722d4

comment:7 Changed 5 years ago by Owen Arnold

  • Status changed from inprogress to verify
  • Resolution set to fixed

This is being verified as pull request #618.

comment:8 Changed 5 years ago by Owen Arnold

refs #11573. Fix compilation issue

Changeset: 0101f10ab7699966901e208c72ee302282f71e10

comment:9 Changed 5 years ago by Owen Arnold

refs #11573. Fix doxygen warning.

Changeset: 1b3e1b160dd389e1d8ed08aca7177cec49b4bc00

comment:10 Changed 5 years ago by Owen Arnold

refs #11573. Fix doxygen warning.

Changeset: 51aced26e65b168c4e5836aefdef0826eb8eae7d

comment:11 Changed 5 years ago by Owen Arnold

refs #11573. Fix doxygen warning.

Changeset: 66c3958121907765147b91f12c39f7b5df73bbec

comment:12 Changed 5 years ago by Owen Arnold

refs #11573. Fix merge conflict. merge master.

Merge branch 'master' of github.com:mantidproject/mantid into 11573_integration

Conflicts:

Code/Mantid/Vates/VatesSimpleGui/ViewWidgets/inc/MantidVatesSimpleGuiViewWidgets/BackgroundRgbProvider.h

Changeset: 3d32d2c79cdf3d2185d3b35666533376ba7d8476

comment:13 Changed 5 years ago by Owen Arnold

refs #11573. Property removed. Update unittests.

Changeset: 4a8f38661b8f53d4b3f659a071df2fa7867ca065

comment:14 Changed 5 years ago by Owen Arnold

refs #11573. Uninitialised variable.

Changeset: 7825817f3e86ff69d67f21f5e259b1dbc89f030f

comment:15 Changed 5 years ago by anonymous

  • Status changed from verify to verifying

comment:16 Changed 5 years ago by Owen Arnold

  • Status changed from verifying to verify

comment:17 Changed 5 years ago by Federico Montesino Pouzols

  • Status changed from verify to verifying
  • Tester set to Federico Montesino Pouzols

comment:18 Changed 5 years ago by Owen Arnold

  • Status changed from verifying to verify
  • Tester Federico Montesino Pouzols deleted

comment:18 Changed 5 years ago by anonymous

  • Tester Federico Montesino Pouzols deleted

comment:20 Changed 5 years ago by Federico Montesino Pouzols

  • Status changed from verify to verifying
  • Tester set to Federico Montesino Pouzols

comment:21 Changed 5 years ago by Federico Montesino Pouzols

I'm compiling this (I'm afraid it's rebuillding most of Mantid and it'll take ~1 h) and my gcc gave me these two warnings: ` MDHistoWorkspaceIterator.cpp:223:45: warning: conversion to ‘Mantid::coord_t {aka float}’ from ‘size_t {aka long unsigned int}’ may alter its value [-Wconversion]

coord_t dRound = size_t(dExact + 0.5); Round to nearest bin edge.

MDHistoWorkspaceIterator.cpp:225:20: warning: conversion to ‘gnu_cxx::alloc_traits<std::allocator<long unsigned int> >::value_type {aka long unsigned int}’ from ‘Mantid::coord_t {aka float}’ may alter its value [-Wfloat-conversion]

indexes[d] = dRound;

` The '-pedantic' builds on the CI machines didn't complain apparently, so I'd say is not worth delaying this PR, but just noting it here, as there seems to be more work following on this?

comment:22 Changed 5 years ago by Owen Arnold

refs #11573. Fix type warning

Changeset: 5082ee197e3ed0df9cdf0d91c3bfe044af48d34d

comment:23 Changed 5 years ago by Owen Arnold

refs #11573. Fix type warnings

Changeset: 8830e5096bf6f76d72d34e9fd5e5ee0eb844fd4f

comment:24 Changed 5 years ago by Federico Montesino Pouzols

All looks good and thoroughly tested and documented. I did a few manual tests as in the PR description and it behaved well.

I'm just waiting for the CI builds to finish. There's a bit of a queue for some CI platforms today, so if this is urgent just ping me and I'll just merge it, The last few changes are harmless and this had previously got the green tick.

comment:25 Changed 5 years ago by Owen Arnold

refs #11573. Implies int generic list fixed.

Changeset: 500a691a6e857455450b9c46bec9a8c3f6416e09

comment:26 Changed 5 years ago by Federico Montesino Pouzols

refs #11573, fix extra ';' warning picked by gcc in Ubuntu

Changeset: 1ff27b367e9e74a24888fd8c805bccd2464282cf

comment:27 Changed 5 years ago by Federico Montesino Pouzols

There was a failed build on ubuntu because of a ';' warning that I took the liberty to fix as Owen is away today. And then there's a failed OSX build because of a couple of warnings coming from another, unrelated branch (SaveNXSPETest.h). Ready to merge.

comment:28 Changed 5 years ago by Federico Montesino Pouzols

  • Status changed from verifying to closed

Merge pull request #618 from mantidproject/11573_integration

Add integration algorithm for MDHistoWorkspaces

Full changeset: bf13dd638090989706d901e4de3e0157a5ec2332

comment:29 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 12411

Note: See TracTickets for help on using tickets.