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-3688 split linux/OSX expected results #3

Merged
merged 12 commits into from Sep 8, 2015

Conversation

jhoblitt
Copy link
Contributor

@jhoblitt jhoblitt commented Sep 4, 2015

No description provided.

@jhoblitt jhoblitt changed the title Tickets/dm 3688 tickets/DM-3688 split linux/OSX expected results Sep 4, 2015
@jhoblitt
Copy link
Contributor Author

jhoblitt commented Sep 4, 2015

I seem to have forgotten some of the past discussion on this topic, remind me why we are retaining both the numpy and the numdiff comparison methods?

@jdswinbank
Copy link
Contributor

Russell asked the same thing. My answer to him was:

Why support both numdiff and numpy? It would be useful to document the reason for that. If numpy suffices it would be nice to stick with it, since it requires no extra dependencies.

I had two motivations here:

  • While developing, I wanted to run both in parallel to demonstrate I was getting the same numbers;
  • I wanted to be as minimally invasive as possible on the CI system, since I have no way to test the impact of my changes there. In particular, CI jumps through hoops to build and install numdiff for this purpose, and I didn't want to start trying to unpick that.

I'm happy to drop the numdiff support if both you & Joshua Hoblitt reckon it's unnecessary overhead. Do note, though, that it's not really a "dependency", in that the script will run fine without it unless you specifically request that numdiff be used to perform the comparison.

Given that you're asking the same thing, maybe we should disable numdiff. I've no problem with pulling it from the script here; I'll see if it's obvious what I need to do to stop Jenkins building it.

@jhoblitt
Copy link
Contributor Author

jhoblitt commented Sep 8, 2015

@jdswinbank That sounds reasonable to me. Lets give this a try but leave the numdiff installation in lsstsw/bin/deploy for the time being so its easy to back out.

Testing is a bit difficult as lsst_build requires a working eups installation. I test by installing buildbot-scripts and lsstsw then running lsstswBuild.sh.

called "output_small".

By default, we use some bespoke Python code to perform the comparison. Specify
the ``--use-numdiff`` option to ``bin/compare`` to use `Numdiff
Copy link
Contributor Author

Choose a reason for hiding this comment

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

References the now removed --use-numdiff flag.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch, thanks.

@jdswinbank jdswinbank merged commit 5eedb13 into master Sep 8, 2015
@ktlim ktlim deleted the tickets/DM-3688 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

2 participants