← 输入框|github.com|刷新
Skip to content

Improve mechanism for documenting ops #50

Open

Description

@ctrueden

It is crucial that we have a robust, simple, and integrated mechanism for documenting ops.

Documenting specific implementations

Our current plan/idea is to use javadoc on the method/field/class implementing it. (It makes most sense for classes and methods; we'll need to think carefully about the best way to document parameters of lambdas assigned to fields.) The main challenge for this is that javadoc is not retained in bytecode. Possible solutions:

  • An annotation processor that executes as compilation time to scrape the javadoc and record it into some metadata inside the main JAR file—e.g. therapi-runtime-javadoc
  • Scraping the -javadoc classifier JAR on demand for the content. But then we'd need to download these from the remote on demand and cache them somewhere and parse it. But this could create a substantial delay the first time help is called, or upon the first op lookup failure.

Algorithms as a whole

We also need to document algorithms as a whole—e.g. what does filter.sobel do mathematically? (Thanks @marktsuchida for suggesting this.) Each filter.sobel op implementation can then lead with a link to the general algorithm documentation for overview, and then focus on any implementation-specific notes as needed.

I don't currently have a great idea where this general documentation would be placed, technically. If we do go with javadoc for documenting implementations, we would need another class somewhere per op name, which seems suboptimal. Maybe package-info.java in packages?

Possible syntax

Syntax for getting help on a specific algorithm could be:

ops.op("math.add").inputs(in1, in2).outputs(out).help()

Syntax for accessing the general documentation could be:

ops.help("math.add")

or:

ops.op("math.add").help()

as well as algorithms as a whole (thanks @marktsuchida for suggesting.

Activity

  1. ctrueden commented on Apr 29, 2021

    @ctrueden
    MemberAuthor
  2. added this to the 1.0.0 milestone on Apr 29, 2021
  3. gselzer commented on May 13, 2021

    @gselzer
    Member

    Time commitments:

    Documenting specific implementations

    Complete, see scijava/incubator#28

    Algorithms as a whole

    Likely 1 week, once a strategy has been devised.

  4. gselzer commented on May 13, 2021

    @gselzer
    Member

    Algorithms as a whole

    We also need to document algorithms as a whole—e.g. what does filter.sobel do mathematically? (Thanks @marktsuchida for suggesting this.) Each filter.sobel op implementation can then lead with a link to the general algorithm documentation for overview, and then focus on any implementation-specific notes as needed.

    I don't currently have a great idea where this general documentation would be placed, technically. If we do go with javadoc for documenting implementations, we would need another class somewhere per op name, which seems suboptimal. Maybe package-info.java in packages?

    @ctrueden @marktsuchida upon reflection, I am starting to think YAGNI here. Do we really need to describe what filter.sobel does when a simple web search could do the trick? How do we go about documenting collections of Ops whose shared name means different things in different contexts? For example, math.multiply could refer to scalar or matrix multiplication, are we going to explain both paradigms in this context? If I were my own devil's advocate, I'd say that two different contexts should require two different names, but this would work against the flexibility of ops.

    Furthermore, I am just as stumped as @ctrueden about the location in which this documentation would live. I agree that it would make sense to have a class per Op name, but this seems like a lot of boilerplate to add. It also reminds me of the interfaces we implemented in ImageJ Ops

    So I'd suggest that we punt on this for now. Honestly, this documentation might be best suited for a location outside of the Scijava Ops codebase, like the ImageJ wiki or ImageJ tutorials. Happy to discuss further with you on this.

  5. marktsuchida commented on May 13, 2021

    @marktsuchida

    @gselzer Here are my thoughts.

    • If new users of Ops are expected to choose an op by name and not worry about how implementations are selected under the hood, it probably makes sense for there to be documentation for each op name.

    • It might not be needed for multiplication or a Gaussian filter, but many algorithms have subtle variants (e.g. normalization factors of FFT and IFFT), or similar names for different algorithms, and it can be worth clarifying what the exact math is (not necessarily as a full writeup; often a few words may suffice for many standard functions). The intent was not to actually explain what matrix multiplication is, although saying that the op performs matrix multiplication as opposed to element-wise multiplication could be quite useful to a newcomer -- either NumPy users or MATLAB users will be surprised depending on which way it is.

    • Much more specialized algorithms (including entirely new ones that do not show up in a search) can be implemented in Ops, in which case it would be nice to be able to describe it and/or link to a paper that describes it.

    • It should also be noted that mathematically equivalent operations aren't always numerically equivalent, so using an op can be problematic if it is not clear what properties are common to all its implementations. To use a slightly artificial example, say there was an op that solves quadratic equations. If I'm faced with just a "solve_quadratic" op, with no further documentation, I would feel compelled to check the docs (or code) of every possible implementation that might be selected to see if they guarantee a certain level of numeric stability, i.e., they don't just naively use the formula we learned in school. If there was documentation on the "solve_quadratic" name itself that states that such guarantees are required for all implementations, I could go straight ahead and use it (modulo implementation bugs, of course).

    At least that is the thinking where this came out of. I don't have a strong opinion on where/how to implement this or how high a priority this should be.

    Just as an idea (not sure how practical this is), I can imagine that one alternative to having a central location for generic docs would be to allow a "canonical" implementation of an op to provide the generic documentation. For the basic functions, one of the implementations provided by SciJava would provide the generic docs. This arrangement would simplify things for third-party ops where there will likely only ever be a single implementation; alternative implementations would still be possible and they would be obligated to behave as the generic docs specify (or else propose an update to the generic docs). The system would presumably raise an error if more than one implementation tried to contribute the generic docs, as this would indicate programmer error. Lots of details to think through, though.

  6. gselzer commented on May 13, 2021

    @gselzer
    Member

    Thanks for the detailed reply @marktsuchida, really appreciate it! I love hearing input from people who aren't involved in day-to-day ops decisions! I'll try to respond to each of your points:

    • If new users of Ops are expected to choose an op by name and not worry about how implementations are selected under the hood, it probably makes sense for there to be documentation for each op name.

    I'm having trouble thinking of a situation where this would be useful. Can you give me an example? Also see the next point (going out of order for response clarity)

    • It should also be noted that mathematically equivalent operations aren't always numerically equivalent, so using an op can be problematic if it is not clear what properties are common to all its implementations. To use a slightly artificial example, say there was an op that solves quadratic equations. If I'm faced with just a "solve_quadratic" op, with no further documentation, I would feel compelled to check the docs (or code) of every possible implementation that might be selected to see if they guarantee a certain level of numeric stability, i.e., they don't just naively use the formula we learned in school. If there was documentation on the "solve_quadratic" name itself that states that such guarantees are required for all implementations, I could go straight ahead and use it (modulo implementation bugs, of course).

    I feel like this would be too restrictive. What if we want to add an Op that does not conform to previously defined requirements? Suppose that we wrote a quadratic equation solver that was not numerically stable but made drastic performance improvements over our set of numerically stable quadratic equation solvers. To preserve backwards compatibility, We can't name it "solve_quadratic" because it would break the namespace guarantee, and we shouldn't change the list of guarantees because it would break backwards compatibility. So I'd think we have to name it something different, making the performance improvements lost to any who aren't in the loop about this new op but are okay with taking the accuracy hit. I'd rather give users the option to control the matching as much as they want (this was the inspiration for scijava/incubator#26). What do you think?

    • It might not be needed for multiplication or a Gaussian filter, but many algorithms have subtle variants (e.g. normalization factors of FFT and IFFT), or similar names for different algorithms, and it can be worth clarifying what the exact math is (not necessarily as a full writeup; often a few words may suffice for many standard functions). The intent was not to actually explain what matrix multiplication is, although saying that the op performs matrix multiplication as opposed to element-wise multiplication could be quite useful to a newcomer -- either NumPy users or MATLAB users will be surprised depending on which way it is.

    This is an argument for annotating the individual Ops directly, no? Suppose we documented everything in some parent class. If a new algorithm is devised for gaussian filtering that does not conform to the specifics described in the parent class, we'd probably have to give this Op a new name just like for the above point, with the same ramifications.

    • Much more specialized algorithms (including entirely new ones that do not show up in a search) can be implemented in Ops, in which case it would be nice to be able to describe it and/or link to a paper that describes it.

    I think this is another argument for annotating the individual Ops directly? If there is a specialized algorithm, you are right that it should be described, but that description belongs on the Op itself, it has nothing to do with the Ops that belong to its same namespace.

    Just as an idea (not sure how practical this is), I can imagine that one alternative to having a central location for generic docs would be to allow a "canonical" implementation of an op to provide the generic documentation. For the basic functions, one of the implementations provided by SciJava would provide the generic docs. This arrangement would simplify things for third-party ops where there will likely only ever be a single implementation; alternative implementations would still be possible and they would be obligated to behave as the generic docs specify (or else propose an update to the generic docs)

    This is a cool idea, but I think that again this restricts the ability of third-party users to innovate. Happy to further brainstorm on this though, and I like that this idea does not add extra classes to the project 😁

    Pleas let me know what you think, and again I am ecstatic that you are taking the time to help us think this through.

  7. marktsuchida commented on May 14, 2021

    @marktsuchida

    I see your point, and the quadratic equation example might not have been the best. There is certainly a tradeoff between ease of extension and ease of knowing what happens when you call a given op.

    I should also say that I am not following scijava-ops development enough to be a good judge of what is best here. This idea came up during a discussion with @ctrueden quite a while back (I forget what about), and I'm just offering my perspective.

    This is an argument for annotating the individual Ops directly, no?

    As I wrote, there is probably not much to specify for a Gaussian filter, and I agree that an overly restrictive specification would be bad. The generic docs should not describe what happens to be common to current implementations, but should describe what the name filter.gauss is intended to encompass. (This is an ideal, of course, and in practice I would expect that any generic doc would get refined over time as new implementations are proposed.)

    It basically comes down to, as a user trying to perform numerical computations, what is it about a given op name that I am able to rely upon, without manually selecting an implementation. If the normalization factor on my FFT may change depending on which implementation gets selected, then I would have no choice but to pin the implementation. If that is not a concern, then it must be that there is an unstated rule that, e.g., the scaling on fft is 1 and on ifft it is 1/N (the most common), but surely it would be better if this were made explicit, both for users and for people adding new implementations (not least because not everybody realizes that the scaling factor can be different between implementations)?

    And I'm not so sure that having to define a new op name is always a bad thing. Supposing that fft has the above scaling rule, it wouldn't be bad if fftOrthoScaled is newly created for implementations that only know how to scale by 1/sqrt(N), if ever encountered. (fft can also gain implementations that take an extra parameter to select the scaling mode; the two are not mutually exclusive.) There can even be an fftArbitraryScaling, where all of the above can be listed (i.e. the same numeric code can appear under this op name), and which would be used by code that doesn't care what the scaling is (perhaps because the result is going to get rescaled to some min and max anyway). And fft (without the scaling choice parameter) would always do the most common thing, in fulfillment of the principle of least surprises. (The example op names are highly bikesheddable.)

  8. gselzer commented on May 14, 2021

    @gselzer
    Member

    I should also say that I am not following scijava-ops development enough to be a good judge of what is best here. This idea came up during a discussion with @ctrueden quite a while back (I forget what about), and I'm just offering my perspective.

    Don't worry, I think this discussion is rather helpful!

    The generic docs should not describe what happens to be common to current implementations, but should describe what the name filter.gauss is intended to encompass.

    Hmm, this is interesting. What might be something that the name filter.gauss is intended to encompass?

    If the normalization factor on my FFT may change depending on which implementation gets selected, then I would have no choice but to pin the implementation. If that is not a concern, then it must be that there is an unstated rule that, e.g., the scaling on fft is 1 and on ifft it is 1/N (the most common), but surely it would be better if this were made explicit, both for users and for people adding new implementations (not least because not everybody realizes that the scaling factor can be different between implementations)?

    I think I am in agreement here. I think that an unstated rule is very bad, and I agree that explicitly stating the rule is better. I am still not convinced, however, that stating that rule explicitly is a good solution. I still don't like the restrictions that puts on us. Suppose we explicitly state that the scaling on filter.fft Ops is 1. Suppose we then write an Op which takes the scale as an argument. Can we call that Op a filter.fft Op? I don't think so, because the scaling is not 1; it is not yet defined.

    And I'm not so sure that having to define a new op name is always a bad thing. Supposing that fft has the above scaling rule, it wouldn't be bad if fftOrthoScaled is newly created for implementations that only know how to scale by 1/sqrt(N), if ever encountered. (fft can also gain implementations that take an extra parameter to select the scaling mode; the two are not mutually exclusive.) There can even be an fftArbitraryScaling, where all of the above can be listed (i.e. the same numeric code can appear under this op name), and which would be used by code that doesn't care what the scaling is (perhaps because the result is going to get rescaled to some min and max anyway). And fft (without the scaling choice parameter) would always do the most common thing, in fulfillment of the principle of least surprises. (The example op names are highly bikesheddable.)

    Yes, this is a great case of using different names to refer to different implementations, and this is the paradigm that Ops currently uses. But in such a case, should we not just create one Op with a scaling argument? (Maybe a wrapper as well, which has a default scaling of 1?) I agree that if the underlying math is different while the set of arguments remains the same, different names are the correct solution here. But then I don't see why you need generic documentation for each of these names; why can't we just annotate these Ops directly?

  9. marktsuchida commented on May 17, 2021

    @marktsuchida

    @gselzer I don't see a problem with the generic docs stating that the scaling is specified by a parameter, but for implementations lacking a scaling-selection parameter, the scaling is 1. The problem only arises when multiple implementations without a scaling-choice parameter differ, or have the potential to differ, in scaling factor.

    In fact, in some ways the generic docs should theoretically be per-(name, signature) rather than just per-name. But that seems way overkill given that ops that differ in signature but not name ought to have much in common.

    I think the situation is similar with the boundary handling (OutOfBoundsFactory) with Gaussian and other filtering operations. Some implementations may have an input for the boundary behavior, but it becomes a question what the behavior is for implementations that lack that parameter. If the rule is that the behavior is implementation-dependent, that is fine -- as long as it is documented somewhere.

    Also, ops are matched based on name and signature, but the signature does not include the parameter names, only their types, correct? So it would seem that some entity should be responsible for documenting the order and meaning of the parameters of the same type when it is not obvious. The primary inputs and outputs are usually obvious, but auxiliary integer and boolean parameters often are not. I can see documenting this being a problem for adding new implementations with new parameters, so I wouldn't go as far as saying that this should be 100% normative, but at least the "known" signatures can be documented to decrease the chances of people coming up with conflicting signatures (those with the same types but different meaning).

  10. gselzer commented on May 17, 2021

    @gselzer
    Member

    Thanks again for your input @marktsuchida!

    I don't see a problem with the generic docs stating that the scaling is specified by a parameter, but for implementations lacking a scaling-selection parameter, the scaling is 1. The problem only arises when multiple implementations without a scaling-choice parameter differ, or have the potential to differ, in scaling factor.

    Yes, you are right that this is a problem. But I feel like the solution you are suggesting here (documenting that all implementations without a scaling parameter use a scaling of 1) is essentially the former of what you described here:

    The generic docs should not describe what happens to be common to current implementations, but should describe what the name filter.gauss is intended to encompass.

    I'm still not sure what the latter means.

    Also, ops are matched based on name and signature, but the signature does not include the parameter names, only their types, correct? So it would seem that some entity should be responsible for documenting the order and meaning of the parameters of the same type when it is not obvious.

    This is what the javadoc is for, yes? And, with our work on scijava/incubator#28, this should be rather easy to provide to the user. Or am I missing something here?

    I'm not sure that you'll be able to get through my thick skull 😁, so I'm just going to try to distill my objections below:

    • Adding generic namespace javadocs adds some burden to developers. Not only do developers need to change the generic docs every time a new Op is added to the namespace, but this will also add an aura of restrictiveness should a developer wish to design an Op that contradicts the specifications outlined in the docs. Of course, a new namespace could fix the issue, but this could hinder the flexibility of Ops.
    • There will be a substantial amount of boilerplate added to the codebase, unless we deem that it is better to have a "canonical implementation" that bears the generic javadoc. This route would have its own problems too, however; I also foresee it being difficult to determine a singular "canonical" implementation.
    • I still think there is an element of YAGNI here. For those who care deeply about the omitted parameters, will this really prevent them from looking at the individual implementations. Would they not be better off calling the implementation that they want directly? I don't know yet.

    I'd like to table the issue for now, maybe I can try implementing something like this later in the summer and see what it looks like and how restrictive it might be. I'd also love to hear from other devs about this, maybe I'm just being too stubborn...

    Thanks again @marktsuchida!

  11. marktsuchida commented on May 17, 2021

    @marktsuchida

    Well, it might be YAGNI in the sense that even the individual implementations are scarcely documented currently (making it rather hard to even come up with concrete examples without e.g. studying the code). I could certainly agree with the idea that those should be made more complete before worrying about what is suggested here.

    I think if the generic docs need to be modified every time an op is added or if they feel restrictive when adding a relatively typical alternative implementation, then it's not being done right, i.e., it's overspecifying. I really only think it should specify the minimum information needed for a user to correctly use the op without pinning the choice of implementation (and yes, determining what 'correctly' entails does require judgement calls from time to time).

    If a proposed new implementation differs in behavior enough that it could not be anticipated when the generic docs were written, doesn't that mean that users of that op probably didn't anticipate it either, and therefore it's a bad idea to add anyway? I guess I'm struggling to see how Ops can be used by third-party projects unless there is some guarantee about what an op is going to compute, at least during the same major version.

    In any case, I think I've explained more or less all of the thoughts I had, so will leave it to the SciJava team to decide what to do.

  12. modified the milestones: 1.0.0, unscheduled on Oct 18, 2023
  13. ctrueden commented on Nov 29, 2023

    @ctrueden
    MemberAuthor

    This is a broad issue, tying into several different pieces of Ops:

    1. How to document an op implementation, on the technical side. We settled on an ops.yaml manifest file for this, with Java-based Ops typically using Javadoc including an @implNote op ... annotation for this. So that part is solved, at least for Java. For Python, we'll presumably scrape function docstrings plus some special annotation analogous to the @implNote op for the op-specific declarations
    2. How to document an op name i.e. all form-equivalent ops. We are tentatively settling on having humans do it wiki-style in an online op index. We need to file a separate issue for this, since there isn't one AFAICT, only a brief mention of it in Convert all Op plugins from ServiceLoader to op.yaml style #140 right now. The discussion above is especially relevant to that piece.
    3. There is also an even more general level, not discussed above, but which we ought to document as well: namespaces. This could also be done on the op index/wiki level too. An alternative idea for this level that @gselzer and I discussed would be to add syntax to the ops.yaml file where documentation for entire namespaces can be added. But I feel like if we do this, we should also have such a thing for entire op names as well, since that is more specific than a namespace. And then there are questions such as how to handle merging of multiple documentation blocks of the same op name or namespace across multiple ops.yaml files present in the environment. So the op index seems like a better place to do this.
    4. How to get help at runtime regarding specific ops, i.e. the .help(...) function. A lot of work on this front has already been done e.g. with Partition Simplification Framework incubator#95, so maybe for the moment we can call this part solved? At least until an op index exists and we start talking about whether ops.help should pull stuff down from the index, and/or whether the web index could be driven by common YAML manifest files that ops.help would also pull down and parse... 😬

    Anyway, that's all a long way of saying @gselzer, I think we should close this issue in favor of filing a new one for the online op index! That will help us to keep things focused and separate concerns conceptually.

  14. added a commit that references this issue on May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

Relationships

None yet

Development

No branches or pull requests

Issue actions