[PIPE2D-33] Sort out Spectrum containers Created: 02/Jul/16  Updated: 11/Oct/16  Resolved: 10/Oct/16

Status: Done
Project: DRP 2-D Pipeline
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major
Reporter: aritter Assignee: aritter
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Sprint: 2014-14, 2014-15, 2014-16
Reviewers: swinbank

 Description   

Currently FiberTraceSet is a std::vector<PTR<FiberTrace>>. For a SpectrumSet I originally had the same concept, but I found that the only way to get rid of the compiler warning "Returning address of a stack object" (something like that) was to go from std::vector<PTR<Spectrum>> to std::vector<Spectrum>. Actually I now have an idea what I might have done wrong (probably created a shared pointer without the "new" in the argument list - should check that) so I think for consistency and to reduce copying I should probably change that back to std::vector<PTR<Spectrum>> and try again...)



 Comments   
Comment by aritter [ 15/Jul/16 ]

Changed SpectrumSet._spectra to PTR(Vector<PTR(Spectrum)>). Tests pass, but I'm still getting a compiler warning:
python/pfs/drp/stella/stellaLib_wrap.cc:4601:9: warning: destination for this 'memset' call is a pointer to dynamic class 'Spectrum<float, unsigned short, float, float>'; vtable po
inter will be overwritten [-Wdynamic-class-memaccess]

Comment by aritter [ 16/Jul/16 ]

We have a similar warning in LSST (ticket DM-1401). I just gonna leave it like that for now...

Comment by swinbank [ 17/Jul/16 ]

Pretty sure the plan was you'd chat to JimB about this, then set it back to "in review" if he doesn't have any good suggestions and agrees that this warning is likely harmless.

Comment by aritter [ 19/Jul/16 ]

Jim Bosh didn't really have anything to say about this. Here is the function that contains the error:

stellaLib_wrap.cc
  template <class Type> 
  struct traits_as<Type, pointer_category> {
    static Type as(PyObject *obj, bool throw_error) {
      Type *v = 0;      
      int res = (obj ? traits_asptr<Type>::asptr(obj, &v) : SWIG_ERROR);
      if (SWIG_IsOK(res) && v) {
	if (SWIG_IsNewObj(res)) {
	  Type r(*v);
	  delete v;
	  return r;
	} else {
	  return *v;
	}
      } else {
	// Uninitialized return value, no Type() constructor required.
	static Type *v_def = (Type*) malloc(sizeof(Type));
	if (!PyErr_Occurred()) {
	  SWIG_Error(SWIG_TypeError,  swig::type_name<Type>());
	}
	if (throw_error) throw std::invalid_argument("bad type");
	memset(v_def,0,sizeof(Type));
	return *v_def;
      }
    }
  };

Seems like the

memset

command is only called if

v

is uninitialized, so as long as our code is correct I believe this should not cause a problem.

Comment by aritter [ 10/Oct/16 ]

Merged into master

Generated at Sat Feb 10 15:47:34 JST 2024 using Jira 8.3.4#803005-sha1:1f96e09b3c60279a408a2ae47be3c745f571388b.