Skip to content

Conversation

@VisualFox
Copy link
Contributor

Rational:

I needed a way to generate half of a Gaussian (and/or its derivative) for implementing a line integral convolution. I can currently use CreateGauss and discard the second half of the kernel but that seem a bit waste given that internally DipLib generate half of a Gaussian anyway.

I'm very aware that a niche use case. So feel free to discard it (as using CreateGauss works). In reality it's mostly a personal excuse to start contributing to DipLib.

Implementation notes:

  • To stay consistent with the way it was originally implemented, I exposed MakeHalfGaussian with the same warning as MakeGaussian, but I kept MakeHalfGaussian implementation hidden behind an unnamed namespace in gauss.cpp (technically just renamed MakeHalfGaussian to MakeHalfGaussian_ and created a new MakeHalfGaussian function) @see Additional notes
  • the new MakeHalfGaussian function check if sigma is set to 0 and return { 1.0 } as do MakeGaussian
  • Added a new bool parameter "full" to CreateGauss and set it to true by default. An alternative will be to create a bool "half" and set it to false by default but it seem less explicit to me.
  • No idea if changing CreateGauss signature will affect any of the binding (Python, Mathlab, Java), an alternative is to either create a new function CreateHalfGauss or make MakeHalfGaussian public
  • I tried to follow the C++ guideline style but let me know if I need to reformat something.
  • Comments may no be great neither, I can rework them as needed.
  • Unit test "make check" passed without any errors.

Additional notes:

I understand the need to hide implementation detail, yet MakeGaussian (and now MakeHalfGaussian) seem quite stable and that will be my preferred way to generate a full (or half) Gaussian kernel as I don't need any of the extra features that CreateGauss bring.

@crisluengo
Copy link
Member

This will affect the Python bindings, because of the introspection method that disambiguates overloaded functions, you need to explicitly give all function parameters. See pydip/src/generation.cpp at line 108 and 110.

…ol in CreateGauss, udate pydip signature for CreateGauss
@VisualFox
Copy link
Contributor Author

VisualFox commented Aug 29, 2025

This will affect the Python bindings, because of the introspection method that disambiguates overloaded functions, you need to explicitly give all function parameters. See pydip/src/generation.cpp at line 108 and 110.

That one is done, but now I clearly see why that probably not a good idea to change public function signature. Now that I understand the problem a bit better it seem that other files may need to be updated as well:

  • pydip/src/documentation_strings.h at line 1973
  • pydip/src/documentation_urls.py at line 2015
  • pydip/src/generation.cpp at line 109 and 110 (doc_strings) - line 108 and 110 should be fixed now
  • dipimage/derivative.m at line 39
  • dipimage/gaussf.m at line 35

@crisluengo
Copy link
Member

  • pydip/src/documentation_strings.h at line 1973
  • pydip/src/documentation_urls.py at line 2015
  • pydip/src/generation.cpp at line 109 and 110 (doc_strings) - line 108 and 110 should be fixed now
  • dipimage/derivative.m at line 39
  • dipimage/gaussf.m at line 35

Don’t worry, I can fix these files tonight when I’m back home. Some of them are auto-generated.

@VisualFox
Copy link
Contributor Author

Don’t worry, I can fix these files tonight when I’m back home. Some of them are auto-generated.

Thank you! Let me know if I can help.

Unit testing on DipLib is still a bit of a mystery for me but I created a quick test file

#include <iostream>

#include <diplib.h>
#include <diplib/linear.h>
#include <diplib/color.h>
#include <diplib/generation.h>

void testMakeGaussian(const dip::dfloat sigma, const dip::uint derivativeOrder) {
    const std::vector<dip::dfloat> kernelFull = dip::MakeGaussian(sigma, derivativeOrder, 3, dip::DT_DFLOAT);
    // std::cout << "kernelFull: " << kernelFull.size() << '\n';
    // for (const auto& v : kernelFull) {
    //     std::cout << v << '\n';
    // }
    //
    // std::cout << '\n';

    const std::vector<dip::dfloat> kernelHalf = dip::MakeHalfGaussian(sigma, derivativeOrder, 3, dip::DT_DFLOAT);
    // std::cout << "kernelHalf: " << kernelHalf.size() << '\n';
    // for (const auto& v : kernelHalf) {
    //     std::cout << v << '\n';
    // }
    //
    // std::cout << '\n';

    bool success = true;
    for (int i=0;i<kernelHalf.size();i++) {
        if (kernelHalf[i] != kernelFull[i]) {
            success = false;
            break;
        }
    }

    if (!success) {
        std::cout << "MakeHalfGaussian ERROR: " << sigma << " " << derivativeOrder << std::endl;
    } else {
        std::cout << "MakeHalfGaussian PASSED: " << sigma << " " << derivativeOrder << std::endl;
    }
}

void testCreateGauss(const dip::FloatArray& sigmas, const dip::UnsignedArray& derivativeOrder) {
    const dip::Image kernelImage = dip::CreateGauss({1.0}, {3}, 3, {0}, "full");
    // std::cout << kernelImage << '\n';

    const dip::Image kernelHalfImage = dip::CreateGauss({1.0}, {3}, 3, {0}, "half");
    // std::cout << kernelHalfImage << '\n';

    bool success = true;
    dip::ImageIterator< dip::dfloat > itHalf(kernelHalfImage);
    do {
        if (const dip::UnsignedArray& coords = itHalf.Coordinates(); ! (kernelHalfImage.At(coords) == kernelImage.At(coords)) ) {
            success = false;
            break;
        }
    } while( ++itHalf );

    if (!success) {
        std::cout << "CreateGauss ERROR: " << sigmas << " " << derivativeOrder << std::endl;
    } else {
        std::cout << "CreateGauss PASSED: " << sigmas << " " << derivativeOrder << std::endl;
    }
}

int main(int argc, char** argv) {
    testMakeGaussian(1.0, 0);
    testMakeGaussian(1.0, 2);

    testCreateGauss({1.0}, {0});
    testCreateGauss({1.0}, {2});
    testCreateGauss({1.0, 3.0, 1.0}, {0, 1, 2});

    return 0;
}

@crisluengo
Copy link
Member

@VisualFox Did you see the PR I made to your PR on your fork? It fixes the docstring stuff.

@VisualFox
Copy link
Contributor Author

@VisualFox Did you see the PR I made to your PR on your fork? It fixes the docstring stuff.

no sorry totally missed that, going to check it. Thank you!

@VisualFox
Copy link
Contributor Author

@VisualFox Did you see the PR I made to your PR on your fork? It fixes the docstring stuff.

Merge is done, committed and pushed. Now waiting for Github automatic testing. Thank you very much for your help and the PR (that was a funny work around)

@crisluengo crisluengo merged commit e0cc55e into DIPlib:master Aug 31, 2025
7 checks passed
@VisualFox
Copy link
Contributor Author

Thank you very much for your help, time and your work on DipLib

@crisluengo
Copy link
Member

@VisualFox Thank you for improving the project. Your contributions are more than welcome!

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.

2 participants