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
danarest plugin crashes on os8 with memory error #635
Comments
I can confirm your observation. I do not recommend running with 8 threads on jlabl5. ---- edit: found a mistake on my part |
Did it run correctly single-threaded? The idea seems familiar but I cannot remember for certain, it was so long ago. |
Valgrind does not tell me much more:
|
This is a hint: This looks for the properties of the target
but this field is not present in the input |
And indeed, skipping over this line prevents the crash: |
Apparently, calling std::vector::front on an empty container causes undefined behavior. Since this is used everywhere in hddm-cpp, I want to hand this problem off to @rjones30 |
Alec, yes calling vector::front on an empty container causes unallocated
memory access, which is a bug. Looking at it now.
-Richard Jones
…On Wed, Dec 14, 2022 at 2:40 PM Alex Austregesilo ***@***.***> wrote:
Apparently, calling std::vector::front on an empty container causes
undefined behavior. Since this is used everywhere in hddm-cpp, I want to
hand this problem off to @rjones30 <https://github.com/rjones30>
—
Reply to this email directly, view it on GitHub
<#635 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3YKWHJGLE33LZEZP6VHS3WNIPCRANCNFSM6AAAAAAQBUALGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hello Alex and all, The properties tag under "target" is not optional in hddm_s. You don't have to specify a <target> but if you do then you must give it <properties>. This is clear from the hddm template, as shown below. If the tag is optional then it has a minoccurs="0" attribute to indicate you can leave it out without breaking the model. Please mark this issue as resolved. I will check to see if it needs to be opened in hdgeant or hdgeant4 to make sure the outputs are compliant.
|
Ok, that makes sense. I can also confirm that the generator gen_amp and its derivatives fill the field for the target. @nsjarvis Where did the input file (etapipi-100.hddm) come from? Can you please make sure it is recent? |
Nevertheless, we can make our lives easier by making the reader more robust against inadvertent omissions that make little difference to the analysis like this. I have posted a PR named fix_hddm_read_crashes_rtj that I think should solve this issue. Please test and if it works, approve. -Richard |
Thank you for fixing this. |
PS I checked with a real data file (hd_rawdata_073070_001.evio) and it runs over that no problem. |
The files necessary to reproduce this are in
/work/halld/njarvis/bug_danarest - use the script run.sh in that directory
It works on ifarm (rhel7) but not on jlabl5 (rhel8)
The screen output from jlabl5 is in /work/halld/njarvis/bug_danarest/os8.out.
Key excerpts from the error messages are
The text was updated successfully, but these errors were encountered: