Ticket #8594 (closed: fixed)
Add option to run ConvertToHistogram to LoadDaveGrp
Reported by: | Martyn Gigg | Owned by: | Samuel Jackson |
---|---|---|---|
Priority: | major | Milestone: | Release 3.2 |
Component: | Indirect Inelastic | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Tester: | Michael Reuter |
Description (last modified by Samuel Jackson) (diff)
As above, but histogram output is optional.
Attachments
Change History
comment:1 Changed 7 years ago by Martyn Gigg
This enables the output to be integrated straight after loading.
comment:2 Changed 7 years ago by Nick Draper
- Owner set to Samuel Jackson
- Status changed from new to assigned
comment:3 Changed 6 years ago by Samuel Jackson
- Status changed from assigned to inprogress
Refs #8594 Catch bad lexical cast.
The original code could cause an (effectively) infinite loop when a bad file format was used.
Changeset: 3efcb667c57c2e85f542e33ed9239c027a1c5f97
comment:4 Changed 6 years ago by Samuel Jackson
Refs #8594 Convert output of LoadDaveGrp to histogram data.
Changeset: 0af1243562b22a95cd4ac7e6b56df375123941ed
comment:5 Changed 6 years ago by Samuel Jackson
Refs #8594 Update unit test to match output.
Changeset: cb59b6c7fc009fcec78a57cb7b53e726f2218e00
comment:6 Changed 6 years ago by Samuel Jackson
Refs #8594 Throw runtime error instead.
This is so that we can catch the error if setRethrows is set to true.
Changeset: 0bb372886e10ceeb161c6a48ac80b6954899e82e
Changed 6 years ago by Samuel Jackson
- Attachment DMPC_295Kdyn_bad.txt added
Testing file which will reproduce the error.
comment:7 Changed 6 years ago by Samuel Jackson
- Status changed from inprogress to verify
- Resolution set to fixed
To Test:
There are two things that need testing:
- Check that the output from LoadDaveGrp is now a histogram.
- The supplied DMPC_295Kdyn_grp can be used to test this. Download and rename to DMPC_295Kdyn.grp
- Check that the algorithm will no longer enter into an (effectively) infinite loop when parsing a bad file format.
- Use the DMPC_295Kdyn_bad.txt to test this. I suggest you run the alg without the changes first to see the problem, then retest with changes and observe that the error is caught.
comment:9 Changed 6 years ago by Michael Reuter
- Status changed from verify to verifying
- Tester set to Michael Reuter
comment:10 Changed 6 years ago by Michael Reuter
- Status changed from verifying to reopened
- Resolution fixed deleted
The ticket specified that making the DAVE grouped ASCII files a histogram was to be an option, not the default behavior. This changes the expectations our scientists have that use this file type. Since NX*NY=NZ, the DAVE grouped ASCII is inherently density data. There is also no guarantee that the NX,NY points can be converted to histogram bins especially in the edge bins. Also, when converting to histogram, if the original file is density data, the bin width needs to be applied to the NZ data when converting to histogram. I'm fine with having the option, but the default behavior should be restored.
comment:11 Changed 6 years ago by Samuel Jackson
- Status changed from reopened to inprogress
Refs #8594 Make the conversion optional.
Changeset: 0bd6954693ef86a9282fa60b997d6732b2f6340f
comment:12 Changed 6 years ago by Samuel Jackson
Refs #8594 Update unit tests.
Changeset: 8eb3246d5c20e0664bb62ae65301e974d9b6b371
comment:13 Changed 6 years ago by Samuel Jackson
My fault. Completely failed to read the "option" part of the title. I've made it optional and added a call to ConvertFromDistribution, which (I think) takes into account the bin width as you described.
comment:14 Changed 6 years ago by Samuel Jackson
- Status changed from inprogress to verify
- Resolution set to fixed
- Description modified (diff)
comment:16 Changed 6 years ago by Michael Reuter
- Status changed from verifying to closed
Merge remote-tracking branch 'origin/feature/8594_add_convert2hist_to_LoadDaveGrp'
Full changeset: dda6f6083adc46ed93e6e636a97ebccef03948d8
comment:17 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 9438