sandialabs / SpecUtils

A library for opening, manipulating, and exporting gamma spectral files
GNU Lesser General Public License v2.1
26 stars 9 forks source link

Missing live time argument in Python wrapper `set_neutron_counts` #30

Closed jasonmhite closed 4 months ago

jasonmhite commented 4 months ago

Over in SpecUtils/bindings/python/SpecFile_py.cpp, setNeutronCounts_wrapper is:

  void setNeutronCounts_wrapper( SpecUtils::Measurement *meas,
                              boost::python::list py_counts )
  {
    vector<float> counts;
    boost::python::ssize_t n = boost::python::len( py_counts );
    for( boost::python::ssize_t i = 0; i < n; ++i )
      counts.push_back( boost::python::extract<float>( py_counts[i] ) );

    meas->set_neutron_counts( counts );
  }

On building I get an error

[ 97%] Building CXX object CMakeFiles/SpecUtils.dir/bindings/python/SpecFile_py.cpp.o
<redacted>/SpecUtils/bindings/python/SpecFile_py.cpp:427:38: error: too few arguments to function call, expected 2, have 1
    meas->set_neutron_counts( counts );
    ~~~~~~~~~~~~~~~~~~~~~~~~         ^
/Users/4uh/Research/source_localization/preprocessing/SpecUtils/SpecUtils/SpecFile.h:812:8: note: 'set_neutron_counts' declared here
  void set_neutron_counts( const std::vector<float> &counts, const float neutron_live_time );
       ^
1 error generated.
make[2]: *** [CMakeFiles/SpecUtils.dir/bindings/python/SpecFile_py.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/SpecUtils.dir/all] Error 2
make: *** [all] Error 2

It looks like at some point set_neutron_counts in the parent class was refactored to also take a float live time but this bit wasn't updated. As quick hack, simply changing it to meas->set_neutron_counts( counts, 0.0 ); seems to work and the code compiles.

jasonmhite commented 4 months ago

Obviously the compiler error is only with -DSpecUtils_PYTHON_BINDINGS=1.

wcjohns commented 4 months ago

Thanks for reporting this!

You are exactly right on the cause of this.

I added the second "live time" argument for the python function. If you pass 0, or a negative number for the live time, the gamma real time will be used (assuming there is a gamma spectrum for that record).

At some point I'll hopefully find time to setup automated builds to check for these kinds of issues on commits.

jasonmhite commented 4 months ago

Bit off topic, so I can open a second issue if needed, but do you have any instructions for building the Python bindings? I wanna play with embedding SpecUtils in some code I'm developing to get better format support. I thought I knew how to build the python module, but I just get a segfault when importing the shared library. This is on an ARM Mac building with clang++ and python 3.10, I can provide more details if you think it's a platform issue.

wcjohns commented 4 months ago

Its probably not a platform issue, since I tested on an M1 mac yesterday.

I just added detailed build instructions for macOS in bindings/python/README.md.

But where I have gotten a segfault in the past is when using a different Python installation than boost was built against, which is a bit annoying. To make sure, when you run the bindings/python/test_python.py, use a command like the following, that explicitly uses the correct install of Python:

/opt/homebrew/opt/python3/bin/python3 test_python.py

Let me know if this, or the detailed build instructions help, -will

jasonmhite commented 4 months ago

Thanks, I'll try that out and if I have any problems I'll start a new issue.