Skip to content

Conversation

@ilfreddy
Copy link
Contributor

@ilfreddy ilfreddy commented Nov 5, 2025

Documentation branch ready for the docathon, copied form @Christian48596

Closes #264 when merged.

@ilfreddy ilfreddy marked this pull request as draft November 5, 2025 09:45
@ylvao
Copy link

ylvao commented Nov 6, 2025

Files that need documentation
Put your name to claim task and mark as done when committed

NOTE we need to compare each header file against master branch. In case that there are comments not for documentation in master branch, we should restore them.

Path: mrcpp/src/trees

  • BandWidth (.cpp, .h) Niklas
  • BoundingBox (.cpp, .h) Niklas
  • CornerOperatorTree (.cpp, .h) Niklas
  • FunctionNode (.cpp, .h) Bin Gao
  • FunctionTree (.cpp, .h) Jacopo
  • FunctionTreeVector (.h) Jacopo
  • HilbertPath (.cpp, .h) Luca
  • MultiResolutionAnalysis (.cpp, .h) Ylva
  • MWNode (.cpp, .h) Tarek
  • MWTree (.cpp, .h) Niklas
  • NodeAllocator (.cpp, h) Jacopo
  • NodeBox (.cpp, .h) Niklas
  • NodeIndex (.h) Ylva
  • OperatorNode (.cpp, .h) Luca
  • OperatorTree (.cpp, .h) Jacopo
  • TreeIterator (.cpp, .h) Luca

@ilfreddy
Copy link
Contributor Author

ilfreddy commented Nov 7, 2025

Next folder to be documented: src/functions
Put your name to claim task and mark as done when committed

NOTE we need to compare each header file against master branch. In case that there are comments not for documentation in master branch, we should restore them.

  • AnalyticFunction.h Niklas
  • BoysFunction(.h .cpp) Bin Gao
  • Gaussexp(.h .cpp) Bin Gao
  • GaussFunc(.h .cpp) Niklas
  • GaussPoly(.h .cpp) Niklas
  • Gaussian(.h .cpp) Jacopo
  • JpowerIntegrals(.h .cpp) Evgueny
  • LegendrePoly(.h .cpp) Niklas
  • Polynomial(.h .cpp)
  • RepresentableFunction(.h .cpp) Jacopo
  • function_utils(.h .cpp) Jacopo
  • special_functions(.h .cpp) Jacopo

There are a few functions I am uncertain if my comments are correct. I will
post them on pull request so that someone can review them.

There are also some template specializations in FunctionNode.cpp, which I am
not sure if we need to make doxygen comment for them.
@bingao
Copy link

bingao commented Nov 7, 2025

Hi @ilfreddy @gitpeterwind I have some problems in the documentation of FunctionNode, maybe you, or other people can help me resolve them.

  1. Function evalf(), the cpp file says it evaluates all polynomials defined on the node. But I think the evaluation is performed for the child node, not this node, because this routine calls getFuncChild(cIdx).evalScaling(r), where cIdx = this->getChildIndex(r).
  2. Function dealloc(). I see this routine calls dealloc of NodeAllocator, which frees the memory of the node by calling ~MWNode() and it seems that coefficients are freed only for LooseNode. I am not sure when a node is a LooseNode, so should we document this detail?
  3. Function reCompress(), It is written in FunctionNode.cpp that, "Option to overwrite or add up existing coefficients". I am not sure what it means, in particular I cannot see the function "add up existing coefficients" unless I overlooked.
  4. Functions integrateInterpolating() and integrateValues() have exactly the same comment in cpp file. It is not easy for me to figure out their difference and make appropriate comments for them.
  5. All comments of template functions dot_scaling() are exactly the same in FunctionNode.cpp, a bit boilerplate. But the important thing is, the conjugate is actually taken for bra and ket in case of complex values, instead of only bra. So, I think the comments in cpp code are not correct for the explanation of conjugate.
  6. Template functions dot_wavelet() have the same problem as dot_scaling().

@ilfreddy
Copy link
Contributor Author

ilfreddy commented Nov 8, 2025

Hi @ilfreddy @gitpeterwind I have some problems in the documentation of FunctionNode, maybe you, or other people can help me resolve them.

  1. Function evalf(), the cpp file says it evaluates all polynomials defined on the node. But I think the evaluation is performed for the child node, not this node, because this routine calls getFuncChild(cIdx).evalScaling(r), where cIdx = this->getChildIndex(r).

I believe this has to do with the following design of MRChem. Each function is an adaptive grid. The terminal nodes of the grid are called leaf nodes. They contain only scaling functions (wavelet coefficients are zero at the leaves). But instead we store that information as scaling+wavelet one level up. So if you were to compute function values at a given node (and not for the children) you would miss one level of refinement and get a less precise result.

  1. Function dealloc(). I see this routine calls dealloc of NodeAllocator, which frees the memory of the node by calling ~MWNode() and it seems that coefficients are freed only for LooseNode. I am not sure when a node is a LooseNode, so should we document this detail?

I leve this to @gitpeterwind

  1. Function reCompress(), It is written in FunctionNode.cpp that, "Option to overwrite or add up existing coefficients". I am not sure what it means, in particular I cannot see the function "add up existing coefficients" unless I overlooked.

Here I am a bitt puzzled myself. I know where the overwrite vs add issue comes from: it has to do with operator applications (too complicated to explain here why you want both options). But I agree with you that there should be a flag for that. And it seems to be gone now.

  1. Functions integrateInterpolating() and integrateValues() have exactly the same comment in cpp file. It is not easy for me to figure out their difference and make appropriate comments for them.

I have not checked in detail, but I actually suspect these two functions do the very same thing...

  1. All comments of template functions dot_scaling() are exactly the same in FunctionNode.cpp, a bit boilerplate. But the important thing is, the conjugate is actually taken for bra and ket in case of complex values, instead of only bra. So, I think the comments in cpp code are not correct for the explanation of conjugate.

I believe this has to do with the way we store functions and "flag" the bra side as being comp conjugate. But again, @gitpeterwind can give you a better answer here.

  1. Template functions dot_wavelet() have the same problem as dot_scaling().

DITTO nr.5 :-)

@gitpeterwind
Copy link
Member

2. Function `dealloc()`. I see this routine calls `dealloc` of `NodeAllocator`, which frees the memory of the node by calling `~MWNode()` and it seems that coefficients are freed only for `LooseNode`. I am not sure when a node is a `LooseNode`, so should we document this detail?

Trees have their own memory management. So dealloc will deallocate in the tree but not in the system.
A loose node does not belong to a tree (not much/never used?), so it will deallocated also by the system

5. All comments of template functions `dot_scaling()` are exactly the same in FunctionNode.cpp, a bit boilerplate. But the important thing is, the conjugate is actually taken for bra and ket in case of complex values, instead of only bra. So, I think the comments in cpp code are not correct for the explanation of conjugate.

6. Template functions `dot_wavelet()` have the same problem as `dot_scaling()`.

The bra and ket can have a "soft" conjugate. That means that one should consider the conjugate of their values instead.
By default (bra.conjugate=false , ket.conjugate=false ,) one should take the conjugate of the bra only when computing <bra|ket>.

Note that for the low level the functions, general explanations are required, to explain the strategy/definitions/choices made. Those should be written elsewhere, and give help understand the relationships between the functions, classes etc.

@bingao
Copy link

bingao commented Nov 10, 2025

Thank you, @ilfreddy and @gitpeterwind

There are only two unresolved ones left. One is the documentation of functions integrateInterpolating() and integrateValues(). Peter thought they compute the same thing using different methods. So, someone who wrote these functions could take a look, if the current documentation is correct.

The other is, there are template specializations in FunctionNode.cpp, I am not sure if we need to make documentation for them as well. How do you think, @moorberry :

template double dot_scaling(const FunctionNode<1, double> &bra, const FunctionNode<1, double> &ket);
template double dot_scaling(const FunctionNode<2, double> &bra, const FunctionNode<2, double> &ket);
template double dot_scaling(const FunctionNode<3, double> &bra, const FunctionNode<3, double> &ket);
template double dot_wavelet(const FunctionNode<1, double> &bra, const FunctionNode<1, double> &ket);
template double dot_wavelet(const FunctionNode<2, double> &bra, const FunctionNode<2, double> &ket);
template double dot_wavelet(const FunctionNode<3, double> &bra, const FunctionNode<3, double> &ket);

template class FunctionNode<1, double>;
template class FunctionNode<2, double>;
template class FunctionNode<3, double>;

template class FunctionNode<1, ComplexDouble>;
template class FunctionNode<2, ComplexDouble>;
template class FunctionNode<3, ComplexDouble>;

template ComplexDouble dot_scaling(const FunctionNode<1, ComplexDouble> &bra, const FunctionNode<1, ComplexDouble> &ket);
template ComplexDouble dot_scaling(const FunctionNode<2, ComplexDouble> &bra, const FunctionNode<2, ComplexDouble> &ket);
template ComplexDouble dot_scaling(const FunctionNode<3, ComplexDouble> &bra, const FunctionNode<3, ComplexDouble> &ket);
template ComplexDouble dot_wavelet(const FunctionNode<1, ComplexDouble> &bra, const FunctionNode<1, ComplexDouble> &ket);
template ComplexDouble dot_wavelet(const FunctionNode<2, ComplexDouble> &bra, const FunctionNode<2, ComplexDouble> &ket);
template ComplexDouble dot_wavelet(const FunctionNode<3, ComplexDouble> &bra, const FunctionNode<3, ComplexDouble> &ket);

template ComplexDouble dot_scaling(const FunctionNode<1, double> &bra, const FunctionNode<1, ComplexDouble> &ket);
template ComplexDouble dot_scaling(const FunctionNode<2, double> &bra, const FunctionNode<2, ComplexDouble> &ket);
template ComplexDouble dot_scaling(const FunctionNode<3, double> &bra, const FunctionNode<3, ComplexDouble> &ket);
template ComplexDouble dot_wavelet(const FunctionNode<1, double> &bra, const FunctionNode<1, ComplexDouble> &ket);
template ComplexDouble dot_wavelet(const FunctionNode<2, double> &bra, const FunctionNode<2, ComplexDouble> &ket);
template ComplexDouble dot_wavelet(const FunctionNode<3, double> &bra, const FunctionNode<3, ComplexDouble> &ket);

template ComplexDouble dot_scaling(const FunctionNode<1, ComplexDouble> &bra, const FunctionNode<1, double> &ket);
template ComplexDouble dot_wavelet(const FunctionNode<1, ComplexDouble> &bra, const FunctionNode<1, double> &ket);
template ComplexDouble dot_scaling(const FunctionNode<2, ComplexDouble> &bra, const FunctionNode<2, double> &ket);
template ComplexDouble dot_wavelet(const FunctionNode<2, ComplexDouble> &bra, const FunctionNode<2, double> &ket);
template ComplexDouble dot_scaling(const FunctionNode<3, ComplexDouble> &bra, const FunctionNode<3, double> &ket);
template ComplexDouble dot_wavelet(const FunctionNode<3, ComplexDouble> &bra, const FunctionNode<3, double> &ket);

Note that these template specializations are not defined in the header file, so I am not entirely sure if we should have documentation for them.

@ilfreddy
Copy link
Contributor Author

@bingao:

  1. I would not document template specializaions, unless someone comes with a good argument for doing so...
  2. I see that the comment in FunctionNode.cpp referring to integrateValues is the same as the one for integrateInterpolating. It seems the code is also quite similar. I checked also where they are used, but they are used in a slight different way. ìntegrateInterpolatingis the equivalent ofintegrateLegendreand it is only used inintegrate(). On the other hand integrateValues` is used to integrate a second function (not the one stored in the tree itself...).

Conclusion: although most likely the two methods do exactly the same thing, the intention behind is different and I would be inclined to keep both. Possibly write a note about it in the documentation.

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.

9 participants