Skip to content
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

Tickets/dm 2961 - psfex port #1

Merged
merged 25 commits into from Aug 14, 2015
Merged

Tickets/dm 2961 - psfex port #1

merged 25 commits into from Aug 14, 2015

Conversation

mjuric
Copy link
Member

@mjuric mjuric commented Jul 28, 2015

No description provided.

natelust and others added 11 commits July 20, 2015 16:52
Changed <foo>Exception to <foo>Error
The code is now using gsl solvers in the psf package
This interferes with a variable defined in a proper name space afw::table
psfex.Context variables must be explicitly be cast to a bool from an int
(0,1). This is most likely due to a new version of swig with more explicit
requirements. Removed the afwCG cast, as this is no longer present in the
lsst code base, all calls to getDetector now return the proper type
Previously values->getAsString would return a std::string variable, and
c_str a pointer to that variable, which would get assigned to argval. The
string object had no guaranteed lifetime in this context. GCC was keeping
the string in some kind of buffer so the error was not being caught. Clang
was reusing the memory location so argval[0] was changed to the value of
varval[1] when the loop continued. This fix insures the strings have a
lifetime long enough that psf-readprefs can copy them into its memory.
Updated the unit test to use a larger radius. Cleaned up unneeded code
inherited from HSC
Updated the table file to reflect the code that was moved from
meas_algorithms to meas_base
}
install(){
scons opt=3 install prefix=$PREFIX version=$VERSION
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These shouldn't be necessary -- you should be able to completely remove the eupspkg.cfg.sh file; eupspkg already knows how to build LSST packages that use scons.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed. I figured out it would not be necessary after the fact of creating it, but just had not gone back and removed it. I was playing around with different opt levels and debug for a while.

@@ -256,7 +256,8 @@ def determinePsf(self, exposure, psfCandidateList, metadata=None, flagKey=None):

context = psfex.Context(prefs.getContextName(), prefs.getContextGroup(),
prefs.getGroupDeg(),
psfex.Context.REMOVEHIDDEN if False else psfex.Context.KEEPHIDDEN)
bool(psfex.Context.REMOVEHIDDEN if False
else psfex.Context.KEEPHIDDEN))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two comments:

  • I think the original was more readable (when everything was on a single line). It's good practice to keep the ternary operator if always all on one line; use a regular if if it can't fit.
  • Is this a manual debugging switch (if False will always eval to False, of course)?

@natelust natelust merged commit 6ff9fc8 into master Aug 14, 2015
@ktlim ktlim deleted the tickets/DM-2961 branch August 25, 2018 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants