ITK_MINIMUM_COMPLIANCE_LEVEL != Module not found

Created on 3 Mar 2020  Â·  8Comments  Â·  Source: InsightSoftwareConsortium/ITK

Description

A module blocked by Compliance Level
1) doesn't generate an error during ITK compilation
2) generates an seemingly unrelated error when that module is attempted to be used by package being compiled against that ITK
3) if I am using a build directory from a version of ITK from a few weeks ago and just do a git update master and recompile, the above error does NOT happen, and all appears to work...waiting for me to remove my build directory when the above errors will then appear

Steps to Reproduce

When configuring ITK with the command:
HEAD is now at 1554b66a55 Merge pull request #1262 from SimonRit/shadow_warning_eigen
+ cd ..
+ mkdir ITK-Debug
+ cd ITK-Debug
+ cmake -GNinja -DBUILD_EXAMPLES=OFF -DBUILD_TESTING=OFF -DModule_MinimalPathExtraction=ON -DCMAKE_BUILD_TYPE=Debug ../ITK

No error / warning is ever reported, BUT since MinimalPathExtraction is at Compliance Level 2 and the default Minimum Compliance Level is 4, the MinimalPathExtraction module is not actually built!

However, when I go to compile ITKTubeTK against that ITK and have MinimalPathExtraction as a required component (via find_package), I get the subtle error

CMake Error at /home/vsts/work/1/ITK/CMake/ITKModuleAPI.cmake:78 (message):
  No such module: "MinimalPathExtraction"
Call Stack (most recent call first):
  /home/vsts/work/1/ITK/CMake/ITKModuleAPI.cmake:31 (itk_module_load)
  /home/vsts/work/1/ITK/CMake/ITKModuleAPI.cmake:129 (_itk_module_config_recurse)
  /home/vsts/work/1/ITK-Debug/ITKConfig.cmake:83 (itk_module_config)
  CMakeLists.txt:135 (find_package)

"No such module" is really hard to track down. The error is reported in CMake/ITKModuleAPI.cmake, but the real problem to fix the error is the compliance level test in a different file: CMake/ITKRemoteModuleAPI.cmake.

One hour later, after dashboards were failing but local build was happily working, I figured this out and realized that had to specify -DITK_MINIMUM_COMPLIANCE_LEVEL=1 when compiling ITK.

Recommendations

  • Throw an error if a module is requested that doesn't meet minimum compliance
  • Throw a different error that is descriptive if a module is requested and it is not possible to add it because it doesn't meet the minimum_compliance_level.
Bug

Most helpful comment

We have encountered the same problems with the RTK module and I haven't been able to comment on this change before (sorry @hjmjohnson!). Here is probably a good place. The minimum compliance level is quite a drastic change and after feeling very welcomed with an easy integration of RTK as a remote module in ITK, the feeling is a bit opposite with this new policy. Yet, I fully understand and respect it, it's good to push everyone to improve ITK code, including the remote one. We'll try to meet the new standard to have the Module_RTK option directly available without having to change the ITK_MINIMUM_COMPLIANCE_LEVEL (which I prefer). Personally, I would suggest

  • to start with a lower level than 4 (e.g., 3) in the next release and let the community know that the level will be raised to 4 in the following one. This gives us more time to implement the requested changes for those who want to keep their module visible with the default options.
  • to only display a warning if a module below the minimum compliance level is activated from the command line instead of the current behavior. I also find it a bit strange that the passed option is silently ignored.

In any case, we will adjust and we are still happier with the current situation than with the one before Module_RTK.cmake was added!

All 8 comments

@hjmjohnson

This has actually gotten a bit worse. It is now a block to any of my work involving ITK.

Now it isn't possible, via cmake-gui, to enable any of the modules below the default compliance level (I think that is what is happening - I'm not certain why available remote modules are no longer being listed)

Even after using cmake-gui to lower the ITK_MINIMUM_COMPLIANCE_LEVEL to 1 and then re-configuring, none of the additional modules appear in the list of cmake variables. See the screenshot below. As a result, none of my code that depends on ITK and its remote modules (e.g., MinimalPath...) can compile at this time!!!

MissingModules

@aylward I did not see this 6 days ago... sorry. I'm dealing with ISBI crisis early this week, but will look into fortifying the process later in the week.

I still feel strongly that it is important to find a way to hide/designate the intended quality of the remote modules. Knowing the expected quality and what "should" compile with high confidence is key to allowing the remote modules to act as a testbed for core changes.

I think there is a quick fix that will simply turn everything back on. I'll try to deal with that late tonight.

Hi,

I appreciate what you are trying to do. It is a good idea to pursue.
Perhaps the level limit and the level of each module is currently too
subtle? For example, I believe that compliance level and each Module_XXXX
variables should be a standard (not advanced) cmake variables? Perhaps
all should be listed, but if enabled, an error message is thrown indicating
it is below the compliance level?

s

On Mon, Mar 9, 2020 at 8:21 AM Hans Johnson notifications@github.com
wrote:

@aylward https://github.com/aylward I did not see this 6 days ago...
sorry. I'm dealing with ISBI crisis early this week, but will look into
fortifying the process later in the week.

I still feel strongly that it is important to find a way to hide/designate
the intended quality of the remote modules. Knowing the expected quality
and what "should" compile with high confidence is key to allowing the
remote modules to act as a testbed for core changes.

I think there is a quick fix that will simply turn everything back on.
I'll try to deal with that late tonight.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/InsightSoftwareConsortium/ITK/issues/1675?email_source=notifications&email_token=AACEJL4NYG6KCXV26BZ5B6LRGTUNPA5CNFSM4LAVISYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOG4MNY#issuecomment-596493879,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AACEJL3CFV5CGP55ZQCLAW3RGTUNPANCNFSM4LAVISYA
.

--
Stephen R. Aylward, Ph.D.

Senior Director of Strategic Initiatives

Kitware: Advancing the frontiers of understanding
by developing innovative open-source software platforms
and integrating them into research, processes, and products.

We have encountered the same problems with the RTK module and I haven't been able to comment on this change before (sorry @hjmjohnson!). Here is probably a good place. The minimum compliance level is quite a drastic change and after feeling very welcomed with an easy integration of RTK as a remote module in ITK, the feeling is a bit opposite with this new policy. Yet, I fully understand and respect it, it's good to push everyone to improve ITK code, including the remote one. We'll try to meet the new standard to have the Module_RTK option directly available without having to change the ITK_MINIMUM_COMPLIANCE_LEVEL (which I prefer). Personally, I would suggest

  • to start with a lower level than 4 (e.g., 3) in the next release and let the community know that the level will be raised to 4 in the following one. This gives us more time to implement the requested changes for those who want to keep their module visible with the default options.
  • to only display a warning if a module below the minimum compliance level is activated from the command line instead of the current behavior. I also find it a bit strange that the passed option is silently ignored.

In any case, we will adjust and we are still happier with the current situation than with the one before Module_RTK.cmake was added!

A reasonable expectation when passing -DModule_<RemoteModule> in a command line (non-interactive) configuration is that it will enable <RemoteModule> without any other steps or variables.

That is a great idea! If someone knows the name, they probably know that
they want / need it.

s

On Mon, Mar 9, 2020 at 3:25 PM Matt McCormick notifications@github.com
wrote:

A reasonable expectation when passing -DModule_ in a
command line (non-interactive) configuration is that it will enable
without any other steps or variables.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/InsightSoftwareConsortium/ITK/issues/1675?email_source=notifications&email_token=AACEJL4Q7JN4E3RZ7BEG2KDRGVGDZA5CNFSM4LAVISYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOIXICQ#issuecomment-596734986,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AACEJL622IR3QB4QAJRICKTRGVGDZANCNFSM4LAVISYA
.

--
Stephen R. Aylward, Ph.D.

Senior Director of Strategic Initiatives

Kitware: Advancing the frontiers of understanding
by developing innovative open-source software platforms
and integrating them into research, processes, and products.

I think is has been fixed. Please re-open if the issue persists.

Thanks @hjmjohnson !

Was this page helpful?
0 / 5 - 0 ratings