Skip to content

Conversation

dzenanz
Copy link
Contributor

@dzenanz dzenanz commented Nov 2, 2020

A cleaned-up version of #378. Closes #378.

dzenanz and others added 16 commits September 29, 2020 10:50
Missing typename directives in declarations.
Confusion between long and long long
Use of itkMutexLock instead of std::mutex.
Failure to include <itkMacro.h>.
Use of ComposeRGBImageFilter instead of ComposeImageFilter.
Use of NULL instead of nullptr.
Use of vcl_sqrt instead of std::sqrt, etc.
itk::MultiThreader --> itk::MultiThreaderBase.
Removal of inappropriate Seg3D:: explicit qualification.
M:\Dev\Seg3D\src\Application\LayerIO\Matlab73LayerImporter.cc(32,10): fatal error C1083: Cannot open include file: 'itkhdf5/H5LTpublic.h': No such file or directory

H5 HL library is disabled in ITK's internal configuration:
https://github.com/InsightSoftwareConsortium/ITK/blob/v5.1.1/Modules/ThirdParty/HDF5/src/CMakeLists.txt#L12
registration->StartRegistration() ==> registration->Update()
GetJacobian(...) ==> ComputeJacobianWithRespectToPosition(...)
so we don't need to have it as a dependency.
@dzenanz dzenanz mentioned this pull request Nov 2, 2020
@tarkpate
Copy link

tarkpate commented Nov 4, 2020

Built on Arch Linux w/ Qt 5.15.1

This is the error I get building with default settings.

In file included from /home/tpat/repos/seg3d/src/Application/LayerIO/Matlab73LayerImporter.cc:31:
/home/tpat/repos/seg3d/bin/Externals/Install/ITK_external/include/ITK-5.1/itk_H5Cpp.h:27:11: fatal error: itkhdf5/cpp/H5Cpp.h: No such file or directory
   27 | # include "itkhdf5/cpp/H5Cpp.h"

This is what I get if I try setting ITK_USE_SYSTEM_HDF5=ON in cmake.

/home/tpat/repos/seg3d/src/Application/LayerIO/Matlab73LayerImporter.cc: In member function 'bool Seg3D::Matlab73LayerImporterPrivate::scan_mat_struct(hid_t, H5::Group&, std::string&)':
/home/tpat/repos/seg3d/src/Application/LayerIO/Matlab73LayerImporter.cc:519:40: error: too few arguments to function 'herr_t H5Oget_info3(hid_t, H5O_info2_t*, unsigned int)'
  519 |       if (H5Oget_info(obj_id, &obj_info) < 0)

// Copy the database content from the source
sqlite3_backup* db_backup_obj = sqlite3_backup_init( this->private_->database_,
"main", src.private_->database_, "main" );
if ( db_backup_obj == nullptr )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that DatabaseManager is pure C++ file, so leaving nullptr in here is fine. I will edit this commit.

Copy link
Contributor

@allywarner allywarner Nov 4, 2020

Choose a reason for hiding this comment

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

Anything that has to do with SQL requires NULL and not nullptr. They need to stay NULL or there will be SQL errors. When I was investigate the errors, I found them in DatabaseManager and Project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tried building and running. There are no SQL errors now like there were before.

@dzenanz
Copy link
Contributor Author

dzenanz commented Nov 4, 2020

@RubioJr9 switch from ITK 5.1 to master (provisionally 5.2) breaks the incremental build. You could try cleaning up only ITK-related part of superbuild, but the best way is to make a clean build of Seg3D (configure + build in a new binary directory).

@dzenanz
Copy link
Contributor Author

dzenanz commented Nov 4, 2020

I think this is ready. Should I beautify history by squashing and possibly reordering commits?

@dzenanz dzenanz mentioned this pull request Nov 4, 2020
@dzenanz
Copy link
Contributor Author

dzenanz commented Nov 4, 2020

I created beautified commit history in #384. It is up to you whether to merge this (#382) or #384.

@allywarner
Copy link
Contributor

@dzenanz thank you!

@tarkpate
Copy link

tarkpate commented Nov 4, 2020

I wiped my bin folder and that got rid of the issue I was having. However, I'm getting this error now:

/home/tpat/repos/seg3d/src/Core/ITKCommon/Optimizers/itkImageMosaicVarianceMetric.txx:435:59: 
    error: cannot bind non-const lvalue reference of type 'itk::Transform<double, 2, 2>::JacobianPositionType&' {aka 'vnl_matrix_fixed<double, 2, 2>&'} 
    to an rvalue of type 'itk::Transform<double, 2, 2>::JacobianPositionType' {aka 'vnl_matrix_fixed<double, 2, 2>'}
  435 |       t->ComputeJacobianWithRespectToPosition(point, data.J_);

@dzenanz
Copy link
Contributor Author

dzenanz commented Nov 4, 2020

@RubioJr9 which compiler/OS and their versions are you using?

@tarkpate
Copy link

tarkpate commented Nov 5, 2020

OS: Arch Linux(rolling release so no versioning)
Compiler: gcc 10.2.0

@dzenanz
Copy link
Contributor Author

dzenanz commented Nov 5, 2020

@RubioJr9 By reading this SO question, it seems that data.J_ is not the same type which ComputeJacobianWithRespectToPosition expects, so implicit conversion is creating a temporary. I looked at the code and couldn't quite figure out why. Can you copy-paste the full error message with context (instantiation types etc)?

@tarkpate
Copy link

tarkpate commented Nov 6, 2020

I posted the log in the new PR. Let me know if you need more info.

@allywarner allywarner closed this Nov 10, 2020
@allywarner allywarner added this to the 2.5.1 milestone Nov 10, 2020
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.

5 participants