Ticket #6457 (closed: fixed)

Opened 8 years ago

Last modified 5 years ago

Improve ICAT Searching

Reported by: Owen Arnold Owned by: Jay Rainey
Priority: critical Milestone: Release 3.0
Component: Framework Keywords: ICAT
Cc: Blocked By:
Blocking: #1807, #3827, #6441, #8202, #8207, #8209, #8218 Tester: Samuel Jackson

Description (last modified by Nick Draper) (diff)

The following comments were made during unscripted testing of #6400 prior to release 2.4 of mantid

While the catalogue searching "Works", the default values set for the fields in Mantid are very annoying. The interface does not feel logical. For example, the run start and run end numbers seem both to default to 0. Therefore I get zero results back when doing a keyword search, because It's using hidden values, which I did not set and cannot see. I'm sure this is at the bottom priority of things to fix, but It might make excellent work for the next set of students we get.

I've created a ticket out of this comment and assigned it to N. Draper to ensure that we still have knowledge of this at the time new students arrive.

Change History

comment:1 Changed 7 years ago by Nick Draper

  • Milestone changed from Release 2.5 to Release 2.6

Moved to r2.6 at the end of r2.5

comment:2 Changed 7 years ago by Nick Draper

  • Keywords ICAT added

comment:3 Changed 7 years ago by Nick Draper

  • Component changed from Mantid to Framework

comment:4 Changed 7 years ago by Nick Draper

  • Milestone changed from Release 2.6 to Backlog

Moved to backlog at the code freeze for R2.6

comment:5 Changed 7 years ago by Nick Draper

  • Owner changed from Nick Draper to Jay Rainey
  • Description modified (diff)
  • Milestone changed from Backlog to Release 3.0

Here's some ideas of things to improve

comment:6 Changed 7 years ago by Jay Rainey

This ticket has been resolved in #7639. To verify this is true:

  1. Open Mantid
  2. Click Catalog -> Login (Located in the top menu bar).
  3. Click My Data search. You should be able to view your specific data.
  4. Click Basic search and ensure all fields work correctly.
    1. Type bob into the Keywords field.
    2. Results are returned.

Similar functionality has been implemented for ICat4. To test this on ICat4 you will need to change attributes in the Facilities.xml file. To do this, read comment:25 in my other ticket.

Last edited 7 years ago by Jay Rainey (previous) (diff)

comment:7 Changed 7 years ago by Jay Rainey

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

comment:8 Changed 7 years ago by Keith Brown

  • Status changed from verify to verifying
  • Tester set to Keith Brown

If this was fixed in another ticket, the resolution should be "invalid" But i'll test it anyway

comment:9 Changed 7 years ago by Keith Brown

Again, you've not pushed this to develop i can't start the test

comment:10 Changed 7 years ago by Keith Brown

  • Status changed from verifying to reopened
  • Resolution fixed deleted

The date doesn't work as i would expect. In order to search for a result of a specific day (so from 00:00:00 to 23::59:59 on a single day) I would expect to insert an identical start and end date. but this doesn't happen, instead I'm given no results.

I'd like to be able to use the same date in both fields.

comment:11 Changed 7 years ago by Jay Rainey

  • Status changed from reopened to inprogress

Now produces the correct datetime from a given string. Refs #6457.

Changeset: dedb47e80aef0703e063bd1018f59a0e9a88c7f6

comment:12 Changed 7 years ago by Jay Rainey

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

The problem occurs in the getTimevalue method in CatalogSearchParam.cpp, which does not produce the correct date/time when the user selects a date in the calendar. For example, if you select today, it will give you yesterday. This will be taken care of during maintenance, and will be addressed in #8152.

As a fix for the release I have added the hours/days to the start/end date in order to allow the user to select the correct day, and also allow them to select a specific day (as suggested by Keith above).

To test

This functionality only works on ICAT4. In order to test this you will need to change the Facilities.xml to:

<catalog name="ICat4Catalog">
  <soapendpoint url="https://icatisis.esc.rl.ac.uk/ICATService/ICAT"></soapendpoint>

To verify that the start/end date produced the incorrect results open Mantid prior to testing and:

  1. Log into the catalog.
  2. Right click the Results log and set the Log level to Debug. This will allow you to view the search query that contains the start/end date in ICAT4.
  3. Load Basic search, and Select a start and end date.
  4. Click Search and Look at the log results. Note how the start/end date are 1 day short of the date you selected, and both times are 23:00:00.

To verify I have addressed the problem obtain my branch/code changes and run through the steps above. The start/end date should now be the correct date. The start time should be: 00:00:00 while the end time should be: 23:59:59.

Last edited 7 years ago by Jay Rainey (previous) (diff)

comment:13 Changed 7 years ago by Keith Brown

  • Status changed from verify to verifying

comment:14 Changed 7 years ago by Keith Brown

  • Status changed from verifying to verify
  • Tester Keith Brown deleted

I seem to get a fatal crash when I've change the xml file from ICAT3 to ICAT4, restart mantid, then try and search.

returning to pool to make sure it's not just my environment

Last edited 7 years ago by Keith Brown (previous) (diff)

comment:15 Changed 7 years ago by Nick Draper

  • Status changed from verify to verifying
  • Tester set to Nick Draper

comment:16 Changed 7 years ago by Nick Draper

  • Status changed from verifying to reopened
  • Resolution fixed deleted

Still crashes on windows, heres the call stack

msvcr110d.dll!_Strftime_l(char * string, unsigned __int64 maxsize, const char * format, const tm * timeptr, void * lc_time_arg, localeinfo_struct * plocinfo) Line 285	C++
 	msvcr110d.dll!strftime(char * string, unsigned __int64 maxsize, const char * format, const tm * timeptr) Line 190	C++
 	MantidKernel.dll!Mantid::Kernel::DateAndTime::toFormattedString(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > format) Line 531	C++
 	MantidICat.dll!Mantid::ICat::ICat4Catalog::formatDateTime(__int64 timestamp) Line 714	C++
>	MantidICat.dll!Mantid::ICat::ICat4Catalog::getSearchQuery(const Mantid::ICat::CatalogSearchParam & inputs) Line 128	C++
 	MantidICat.dll!Mantid::ICat::ICat4Catalog::search(const Mantid::ICat::CatalogSearchParam & inputs, boost::shared_ptr<Mantid::API::ITableWorkspace> & outputws) Line 251	C++
 	MantidICat.dll!Mantid::ICat::CatalogSearch::exec() Line 99	C++
 	MantidAPI.dll!Mantid::API::Algorithm::execute() Line 552	C++
 	MantidAPI.dll!Mantid::API::Algorithm::executeAsyncImpl(const Poco::Void & __formal) Line 1337	C++
 	MantidAPI.dll!Mantid::API::AlgorithmProxy::executeAsyncImpl(const Poco::Void & dummy) Line 116	C++
 	MantidAPI.dll!Poco::ActiveRunnable<bool,Poco::Void,Mantid::API::AlgorithmProxy>::run() Line 85	C++
 	PocoFoundation64d.dll!Poco::PooledThread::run() Line 215	C++
 	PocoFoundation64d.dll!Poco::ThreadImpl::runnableEntry(void * pThread) Line 245	C++
 	kernel32.dll!BaseThreadInitThunk
()	Unknown
 	ntdll.dll!RtlUserThreadStart
()	Unknown

comment:17 Changed 7 years ago by Jay Rainey

  • Blocked By 7637 added

comment:18 Changed 7 years ago by Owen Arnold

  • Blocked By 7637 removed

(In #7637) There is a crash fix that must be sorted out. We also want to be confident that we can specify mac-friendly mount points.

comment:19 Changed 7 years ago by Jay Rainey

  • Blocking 8207 added

comment:20 Changed 7 years ago by Jay Rainey

  • Blocking 8126 added

comment:21 Changed 7 years ago by Jay Rainey

  • Blocking 8209 added

(In #8209) To verify that this now works correctly log into the catalog, and perform a search. The result table will be populated, and the end date is never larger than the start date, and that they generally make sense..

To check this is true you could log into the ICAT4 web-gui.

comment:22 Changed 7 years ago by Jay Rainey

  • Blocking 1807 added

comment:23 Changed 7 years ago by Jay Rainey

  • Blocking 8218 added

comment:24 Changed 7 years ago by Jay Rainey

  • Blocking 8202 added

(In #8202) To test this ticket you must have access to the archives.

  1. Enable debugging in the Results log window.
  2. Open the catalog and perform a search.
  3. Select an investigation to the Datafile table will be populated.
  4. Select a datafile to download, and note the output in the debug console. You will see the path before the transformation (e.g. the windows path), and after (e.g. containing your mounted archive point.

comment:25 Changed 7 years ago by Jay Rainey

  • Blocking 3827 added

comment:26 Changed 7 years ago by Jay Rainey

  • Blocking 6441 added

(In #6441) To verify that my changes have addressed the above issues open the ICAT GUI, and check that what I said above is valid.

comment:27 Changed 7 years ago by Jay Rainey

  • Status changed from reopened to inprogress

comment:28 Changed 7 years ago by Jay Rainey

  • Priority changed from major to critical

comment:29 Changed 7 years ago by Jay Rainey

Microsoft do not support %F %T time format. Refs #6457.

Changeset: 2cb26c2b99624a20fc07060f3eefab261220f6cb

comment:30 Changed 7 years ago by Jay Rainey

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

To verify that my changes fixed the problem of ICAT crashing on windows download an older version of ICAT from jenkins when the crash occurred (to verify it did), and then a newer version ( > 2542 on W7) when my changes have been introduced.

Currently access to ICAT4 is not functioning correctly. However, this ticket can still be tested (icat4 end-point on develop is invalid, but the catalog is set to ICAT4, which is where the changes are).

To verify:

  1. Open Mantid, and click "Search".
  2. Press "Cancel" to login. The ICAT GUI will open.
  3. Perform a search. It should no longer crash.

comment:31 Changed 7 years ago by Jay Rainey

  • Status changed from verify to reopened
  • Resolution fixed deleted

comment:32 Changed 7 years ago by Jay Rainey

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

comment:33 Changed 7 years ago by Jay Rainey

  • Status changed from verify to reopened
  • Resolution fixed deleted

comment:34 Changed 7 years ago by Jay Rainey

The above fix fixed one crash (related to dates), but when I perform a search that produces results I get another crash. Clearly something is going wrong when populating the table. Looking into it now.

Last edited 7 years ago by Jay Rainey (previous) (diff)

comment:35 Changed 7 years ago by Jay Rainey

  • Status changed from reopened to inprogress

Merge branch 'feature/8126_icat4_external_download' into develop. Refs #6457.

Conflicts:

Code/Mantid/Framework/ICat/src/ICat4/ICat4Catalog.cpp

Changeset: b7b8705cb8b3bcaa6a05c9eab9096a14ece0c5f0

comment:36 Changed 7 years ago by Jay Rainey

I accidentally committed to the wrong ticket a fix for the crash noted in comment:34. The above merge to develop was in fact made in #8126.

To rectify this I will download and test a develop build (since it has changes in #8126 that are required to see the crash), and if the above change fixes it then #8126 will need to be tested before this ticket.

comment:37 Changed 7 years ago by Jay Rainey

  • Blocking 8126 removed

comment:38 Changed 7 years ago by Jay Rainey

  • Blocking 8126 added

comment:39 Changed 7 years ago by Jay Rainey

  • Blocking 8126 removed

(In #8126) I am now working on fixing the windows crash noted in #6457 here as I require the ICAT4 endpoint, which has been updated here (previous ICAT4 end-point is no longer valid, and changing the end-point requires regeneration of gSoap files).

comment:40 Changed 7 years ago by Jay Rainey

  • Blocking 1807 removed

comment:38 Changed 7 years ago by Jay Rainey

  • Blocking 1807 added
  • Tester Nick Draper deleted

comment:39 Changed 7 years ago by Jay Rainey

I am going to resolve the windows crash in #8126. To test this ticket see comment:1, and verify that my solution in comment:29 addresses Keith's suggestion in comment:10

Last edited 7 years ago by Jay Rainey (previous) (diff)

comment:40 Changed 7 years ago by Jay Rainey

  • Blocking 1807 removed

comment:41 Changed 7 years ago by Jay Rainey

  • Blocking 8202 removed

comment:40 Changed 7 years ago by Jay Rainey

  • Status changed from inprogress to verify
  • Resolution set to fixed
  • Blocking 1807, 8202 added

comment:41 Changed 7 years ago by Samuel Jackson

  • Status changed from verify to verifying
  • Tester set to Samuel Jackson

comment:47 Changed 7 years ago by Samuel Jackson

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/feature/6457_fix_icat_win_crash'

Full changeset: beb37cd6d390c9857dfa68ee10407cf7ab9f5312

comment:48 Changed 7 years ago by Samuel Jackson

The interface still requires a lot of work but as I can see these issues will be handled in other tickets and the fixed version of the query is showing up I'll mark this as fixed.

comment:49 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 7303

Note: See TracTickets for help on using tickets.