Is this the external one? https://github.com/ndarray/ndarray
Doesn't look too active.

Yup, that's it. I get around to merging contributed PRs probably once every few months, and there hasn't been any changes outside the build system for quite a while. Eventually there will be to get off boost, but that will be hard (because I'm boost::fusion for some tricky stuff).

Don't forget this extremely important pull request... https://github.com/ndarray/ndarray/pull/42

Basically done (barring pushback on the points below). Per preference from John Swinbank I opted to stick the release in a tarball rather then pull it directly from github.
His arguments for this (if I am correct are):

1. The guide (https://developer.lsst.io/build-ci/third_party.html) specifically says to do this.
2. There is no precedence for doing otherwise.

Furthermore, in order to keep development time on this ticket to a minimum I also opted for the following:

1. Use our own LSST `SConstruct` files instead of the ones from upstream (but do add the new tests).
2. Patch array size to use `int` (as in our codebase) instead of `size_t` (as in upstream). First tried the opposite, but this took a long time to hunt down all cases and I wasn't sure if it is worth the effort. Feel free to push back in the review.
3. Patch ndarray to fix a few `class` vs `struct` mismatches. I would be happy to upstream those changes instead. But I'm not sure if this is in scope for the ticket. Plus I don't know what the upstreaming procedure is in this case.

A CI build has successfully completed.

Sorry this ended up being a wild goose chase, but if we're going to use our own SConstruct files and keep using int for array size, I think we're better off just developing on the LSST fork until we're ready to do those changes. AFAIK, there really isn't much else different. Let's just defer this ticket until later.

I agree that we really need to be using at least the same ndarray API even if we have tweaked the build files. Using upstream ndarray but patching the API seems like a bad end state.

Sure, if we think it is worth it I'll hunt down the cases and make it work with size_t. But I'm not quite sure what we gain from using upstream scons?

To flip that question around: what's the downside to the upstream build? Assuming (I've not looked) that it's a well behaved package, we ought to be able to build and install it in some standard location without having to patch it extensively. That's all we need, isn't it?

But I'm not quite sure what we gain from using upstream scons?

Just lower maintenance. That said, I've been meaning to drop upstream SCons in favor of a CMake build anyway, since the upstream one is already running into problems with SCons vs. Python 3, and I don't want to rewrite the upstream SCons build to not assume SCons' Python is the target.

Anyhow, I don't want to create a bunch of work for you on this now; my recommendation that we do this first was predecated on it being a really quick conversion, and it looks like the size_t change is a bit more disruptive than I expected. I realize it's still probably not a ton of work, but I think the rest of the pybind11 conversion is higher priority that getting that done.

a bit more disruptive than I expected

Can we quantify that? I'd actually rather clear this from the backlog if it's practical. If that means Pim does a few hours of grunt work, let's just do it now (sorry Pim); if we're talking days or weeks, then I tend to agree with Jim.

I'd guess 2-3 days:

• The warnings due to signed/unsigned comparisons will be trivial to fix, but maybe hard to find without some hacking of sconsUtils to turn the warning into an error (and it'll be a ton of recompiling).
• The errors due to failure to convert between signed and unsigned will be easy to spot and easy to work around, but I'm a bit worried that these might indicate something that should be fixed within ndarray.
• I don't expect any difficulties switching to the upstream build system, but this might generate a new blocker for switching to Python 3 next month (depending on how Tim Jenness is planning to resolve our own issues with SCons vs. Python 3).

We discussed this at our standup of 2016-07-07.

Pim broadly agrees with Jim's estimate of the likely time requirement.

We agreed to put this on hold while focusing on the pybind11 conversion. If that goes sufficiently well that there's time later in this sprint or in the next, we'll return to this issue.

Can we also clarify the scope of the issue?
In particular can we define the acceptable level of patching (build system / code), if porting to cmake is part of it (in case we adopt upstream build) and wether or not we want to pull directly from github instead of a release tarball?

I'm not sure I understand the comments re build systems. As I understand it, our interaction with the upstream build system should just be to execute it: ie, the equivalent of "configure; make; make install" with some appropriate arguments. That should be independent of whether upstream uses autotools, SCons or CMake. (I guess there could be a problem if we are running on a system which only supplies Python 3, and SCons requires Python 2, but that seems like a second order effect). Why is it more complicated than this?

Using a tarball vs. a direct pull from github seems fairly inconsequential to me. I'd tend to the tarball for consistency and to follow our guidelines, but whatever.

Patching the code should only be for LSST specific modifications that upstream won't accept. I would imagine that they would be very minimal here, unless Jim is hard to please!

Ok, understood. Use upstream as is. The build system complication in this case is that it has to guess paths and the likes. Plus it seems to depend on some non-included things that ship as submodules. Was easier just to reuse the existing one. But I'm fine with calling upstream too. Although Jim mentioned wanting to switch it over to CMake which we might want to do first then to avoid extra work later.

I think the upstream SCons build should pretty much just work (though you will need the submodules).

The trouble with SCons and Python 3 is almost what John Swinbank described, but it's a bit worse than that - the ndarray SCons build (like sconsUtils, I think) assumes the Python running SCons is the one we want to build against, and uses that to retrieve various paths from Python. This is not a problem for now, and I'm not sure it's any worse than using sconsUtils. But unless we're expecting SCons to get to Python 3 before we do, it will cause problems when we do (but maybe nothing beyond the problems we'd have with sconsUtils).

Final CI build is running at the moment, but I don't expect any issues since last full Jenkins build worked and all that changed is now using the tagged upstream release.
The code modifications in afw actually turned out to be less work than expected. Relatively few modifications were needed, it just took a while to find out where to put them (and in particular where not to).
What did take time was the build system. To integrate upstream ndarray as an external I ended up adding a new CMake build system to upstream (as the existing SCons one proved difficult to integrate properly and would require more work for Python 3 in the future). This has been reviewed upstream (by Jim Bosch) and is merged into the 1.1.0 release. The final lsst package contains no patches against upstream.

Did you test it with python 3?

Did you test it with python 3?

I'm pretty sure the answer is no, because external ndarray itself won't build against Python 3. I think that should just be a matter of cherry-picking the changes from DM-7138 to upstream.

Ok, please don't switch us until it does work on Python 3.

Tim Jenness thanks for the reminder. I thought all required changes were upstream, they were not. Also encountered some annoying build issues on macOS and linux. Now fixed. Python 2 & 3 CI runs just completed.

Once upstream is reviewed an merged, so can this.

Upstream only had our patches as of April when I made the pull request.

Review is complete, and upstream is merged (but not tagged).

Merge complete after another successful CI build for python 2 and 3 of the latest tagged ndarray release.

Pim Schellart [X] – Please could you add a brief comment on this to the release notes? https://confluence.lsstcorp.org/display/DM/Data+Release+Production+WIP+F16+Release+Notes

Thanks!

Will do!

Done