search

DeepLearnPhysics / larcv2

MIT License
13 stars 16 forks source link

io_overhaul to finish and make release candidate #1

Closed drinkingkazu closed 6 years ago

drinkingkazu commented 6 years ago

@gustavogx thanks for helping today! I just pulled and got a linking error after this commit A few notes: 0) to see the full compilation test, type "make" @ repo top dir, not in larcv/app/ImageMod (or any subdirectories under top). Linking error won't show up otherwise (since linking is done to combine all packages = subdirectories) 1) I noticed the issue is IOManager::get_data where T was specialized to be a pointer. If you look at this function definition here, you see this function returns a reference. So it's not meant to return a pointer :)

I'm gonna fix this and push soon.

drinkingkazu commented 6 years ago

@gustavogx ok I fixed in this commit and also the next one.

One more thing to note: when accessing data product as a pointer, there is no need to be careful (well, except it's a pointer ;)). But when accessing it as an object reference, we have to explicitly specify "&" to avoid slow copy. Just a remark.

drinkingkazu commented 6 years ago

This is finished https://github.com/DeepLearnPhysics/larcv2/commit/8218c28b0ddcf8a16e3dcfcaadfc400574de544e