Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

diffuse or sharpen: document PDE and its relation to speed and directionality parameters #428

Closed
tpapp opened this issue Feb 14, 2022 · 22 comments
Labels
no-issue-activity No activity on this issue

Comments

@tpapp
Copy link

tpapp commented Feb 14, 2022

It is somewhat difficult to understand how the 1st, 2nd, 3rd and 4th order speed and directionality settings should be interpreted for diffuse or sharpen.

Specifically

  • the manual talks about high and low frequencies, it is unclear if this is a consequence of the implementation or a property of the PDE,
  • gradient and laplacian are used, but the gradient is a vector while the laplacian is a scalar, it is unclear how they mix

I would suggest

  1. Documenting the actual PDE that the module intends to solve, abstracting from the implementation details. While not all users are familiar with PDEs, it would help some people and more importantly they could contribute explanations or examples based on this.
  2. Document the implementation details to the extent they are relevant.
  3. Clarify where frequencies enter this picture: as an implementation detail or something fundamental to the PDE.
@matt-maguire
Copy link
Contributor

matt-maguire commented Feb 14, 2022

The paper describing the basic approach taken by the module in terms of PDEs can be found here:
https://doi.org/10.1007/s11042-010-0601-4

The frequencies mentioned are an addition on top of that algorithm -- the module uses wavelets to split the image into low-band and high-band spatial frequency components, and the 1st/2nd order sliders operate on the low-band frequency portion, and the 3rd/4th order sliders operate on the high-band frequency components.

As you say, most users will not have the mathematical skillset to follow these details, which is why the manual takes a more hand-waving approach.

@elstoc
Copy link
Contributor

elstoc commented Feb 14, 2022

Agreed. I think the place for this sort of information is in the code, which, in this case, contains quite a few comments and links to papers. I would prefer to make this section of the manual less technical, not more -- people already have trouble following some of these new modules.

Alternatively you could reach out to @aurelienpierre, who may be able to provide some additional guidance.

@tpapp
Copy link
Author

tpapp commented Feb 15, 2022

@elstoc: Given the results of a recent survey, more than half of the users have a master's degree or a PhD, so I think it premature to assume that users lack background to get at lease some benefit from the details.

@matt-maguire: My understanding is that the algorithm in the paper was modified for the purposes of this module. Darktable is a highly technical piece of software already. There must be a middle ground between handwaving and reading the source code.

I am not suggesting detailed derivations, just maybe one or two equations.

Frankly, I am a bit surprised that a request for more documentation is met with so much immediate resistance. This is a great module, and a lot of excellent work went into it, but it users need to understand it better to realize its full potential.

@elstoc
Copy link
Contributor

elstoc commented Feb 15, 2022

so I think it premature to assume that users lack background to get at lease some benefit from the details

You could equally argue that those users are capable of getting that information from the code. The documentation has to cater for a wider range of users though, not just the self-selecting sample who are actively engaged in forums/groups and chose to answer the survey (although useful as a general guide, this survey is far from unbiased). There are users who are already confused by the docs and many of them will just stop reading when they encounter such technical detail. I'm not against documenting it though and if there was somewhere else we could put it and provide a link that would be fine. I'd just rather keep such detail out of the main user manual if possible.

Most users want to understand how the controls will affect their image and even though I have a masters in physics I still wouldn't know how to interpret such equations in the context of image editing.

Frankly, I am a bit surprised that a request for more documentation is met with so much immediate resistance

Let's just call it scepticism for now. I'm certainly not willing to put in the work required to understand the module at this level and then translate it into user manual text -- all I've done so far is copy-edit the text provided by the developer and I'm far from qualified to assess its accuracy. Perhaps if you submitted a PR with some suggestions for actual wording we could consider your proposal in more detail.

@tpapp
Copy link
Author

tpapp commented Feb 15, 2022

You could equally argue that those users are capable of getting that information from the code.

One can of course always reconstruct information from the code, especially if it has comments, but the cost is rather high. In addition to the Qin et al (2012) paper, the code also relies on Dammertz et al (2010); so it is not simply a matter of reading a paper to understand what this module does, even at an intuitive level.

I'm certainly not willing to put in the work

Perhaps you misunderstood something, I don't think that anyone asked you to do this specifically.

I opened this issue to start a constructive discussion; no one should feel pressured to jump to doing the work at this point. IMO we should wait for users to chime in about what they would prefer.

@elstoc
Copy link
Contributor

elstoc commented Feb 15, 2022

Ok let's wait for other opinions, though I suspect you might be overestimating the audience of this repository.

@todd-prior
Copy link

I won't pretend to understand the math. I am trying to clarify one thing. The concept of the gradient when talking about it as a reference for low and high frequency and setting the direction sliders...Is that (the gradient) the direction of the light from bright to dark in the regions of low detail and high detail respectively. So to be very simple the low frequency gradient runs from light areas say in a sky or background to darker ones and the same logic for tree branches or some other detailed element in a photo again light to dark in the details.

From there then trying to follow the manual for the directional sliders one and two control low frequency direction wrt high (1) and then low (2) frequency gradients and 3 and 4 the high frequency direction wrt low frequency and high frequency gradients respectively.

If I have that right ( not likely) then when I look at an image I look at regions that are bright moving into darker and define that as the direction of the gradient. Then further considering if that gradient of light is in an area of either high or low frequency, then this would be my reference gradient to decide how to set the direction that I want to weight the diffusion adjustment, ie either with that gradient or perpendicular to it??

Now if I am even close to being correct... I am not sure at this point how to analyse an image and apply that logic with a targeted adjustment...maybe practice.....

So in effect I get the idea of with the gradient or perpendicular , I am just not 100% sure I understand the gradient reference points against which these directions interact with.....if that makes any sense at all...

Despite being clueless I do get great results from tweaking the presets I would just like to see if I could gain a bit better understanding

@Dave22152
Copy link

Speaking only as a user with some rusty technical skills in signal processing, I think the PDE might be interesting, but I don't know that I'd get much out of it unless I could translate the solution terms to the sliders on the module.

I think the manual does a good job describing the module, but the words didn't really connect until I went through the videos demonstrating how each control impacts an image, so I think material that helps me to understand how the settings affect them image would go a lot further.

So I might suggest adding a link that's not behind a paywall for further reading, so that interested readers can pursue the math without further complicating the manual.

@paperdigits
Copy link
Contributor

paperdigits commented Feb 15, 2022 via email

@todd-prior
Copy link

todd-prior commented Feb 15, 2022 via email

@Dave22152
Copy link

Dave22152 commented Feb 15, 2022 via email

@tpapp
Copy link
Author

tpapp commented Feb 16, 2022

Documenting implementation details is an absolute no-go for this type of manual

Note that the my suggestion in the issue is not to document implementation details, unless very relevant for the end result. That said, the current documentation already exposes implementation details (eg sum of speeds should not exceed 1). When using this module, the user is effectively taking care of parametrizing a PDE solver and tuning it for numerical stability (lower speeds, increase iterations; other relevant trade-offs with scale).

Which is fine because the module is very useful as is, but do we really expect users to understand these things by experimentation? The module has a lot of degrees of freedom which makes this difficult (also consider runtime, on most machines feedback is not immediate).

other sources are needed to fully understand the module

"Full" understanding is always an elusive goal, but from the discussions on the forum it is my impression that even partial and practical understanding of this module is difficult for most people.

I am not suggesting that we litter the manual with PDEs and implementation details. But a clear description of what this module does would be great, and math is one way of doing it.

An alternative I can imagine is small example images demonstrating the principles. We already have a great example of this in the manual for the wavelets module.

@aurelienpierre
Copy link
Member

aurelienpierre commented Feb 16, 2022

Please note that the implementation is not documented. I have used wavelets theory, 2nd order PDE theory and multi-grid schemes and combined them into a 4th order PDE using a type of wavelet that turns out to be a Laplacian × a coeff. Not to mention the edge detection based on variance thresholding but applied on Laplacian. It's a turducken of maths.

@tpapp
Copy link
Author

tpapp commented Feb 17, 2022

@aurelienpierre: That's the impression I got from the code. It would be great if you could write it up, possibly as a paper, when time permits.

@tpapp
Copy link
Author

tpapp commented Feb 24, 2022

@flannelhead, now that you have an in-depth understanding of the code and the algorithm after darktable-org/darktable#11217 (:rocket:), I am curious if you have any suggestions for the docs of the module.

@flannelhead
Copy link
Contributor

I'm mostly familiar with the original inpainting paper and the related bits of code (the "inner loop" where the most intensive computation takes place) – this familiarity came mostly from an optimization effort last summer. The paper describes that part pretty well, and there are a couple of other good papers covering similar topics.

However Aurélien added a lot on top of the basic idea in the paper. I'm quite lost honestly when it comes to the wavelet decompositions and solving PDEs using them. This would be something I would like to learn more about, as well.

@github-actions
Copy link

This issue has not had any activity in the past 60 days and will be closed in 365 days if not updated.

@github-actions github-actions bot added the no-issue-activity No activity on this issue label Apr 26, 2022
@tpapp
Copy link
Author

tpapp commented Mar 27, 2023

From forum discussions, I still think that some math in the manual would help.

@github-actions github-actions bot removed the no-issue-activity No activity on this issue label Mar 28, 2023
@github-actions
Copy link

This issue has not had any activity in the past 60 days and will be closed in 365 days if not updated.

@github-actions github-actions bot added the no-issue-activity No activity on this issue label May 28, 2023
@tpapp
Copy link
Author

tpapp commented May 28, 2023

Now that Aurelien no longer participates in the development of Darktable, it is my impression that we have a module which does something no one understands in depth.

At the same time, and perhaps because of this, users find it difficult to navigate the UI of this module (cf #412). Presets are useful, but only mitigate the issue: the module has many parameters and experimenting in such a large parameter space without understanding the underlying algorithm is difficult.

(prompted by the issue activity notice, to keep this active)

@github-actions github-actions bot removed the no-issue-activity No activity on this issue label May 29, 2023
@github-actions
Copy link

This issue has not had any activity in the past 60 days and will be closed in 365 days if not updated.

@github-actions github-actions bot added the no-issue-activity No activity on this issue label Jul 28, 2023
@paperdigits
Copy link
Contributor

The user manual is not a place for code documentation. Not saying this doesn't need to be documented somewhere, but this isn't the right place. Probably the dev wiki is the right place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-issue-activity No activity on this issue
Projects
None yet
Development

No branches or pull requests

8 participants