ACEsuit / Polynomials4ML.jl

Polynomials for ML: fast evaluation, batching, differentiation
MIT License
12 stars 5 forks source link

Some Naming Polls #85

Closed cortner closed 2 months ago

cortner commented 3 months ago

I'd like to poll the developers of this package about some naming issues:

(1) Package name: do we keep Polynomials4ML or change to ACEkernels or something else? Or split off the ACE stuff into a separate package? The context for this question is that some operations we provide are really polynomial bases while others are some very unique tensor operations.

(2) For the polynomial basis interface the terminology is now fairly settled to

evaluate!
evaluate_ed!
evaluate_ed2!

I think this is fine and suggest to keep it unless there are dissenting views.

(3) For the tensors this is less clear I think.

(3.1) Pullbacks Options:

pb_evaluate!
pullback_evaluate! 

(3.2) Double-pullback

pb_pb_evaluate!
pullback2_evaluate! 
pullback_pullback_evaluate!

(3.3) Push-forward

pfwd_evaluate!
pushforward_evaluate! 

Thoughts?

CC @CheukHinHoJerry @DexuanZhou @zhanglw0521

CheukHinHoJerry commented 3 months ago

(1) I think keeping Polynomials4ML is good. It would be easier to maintain if everything about kernels are in the same package?

(3) I like all the shorter ones.

zhanglw0521 commented 3 months ago

Given that this is a poll I guess I'd just express my thoughts -

(1) I think this package could even be used outside the ACE community so it might be a good idea not to make its name ACE-related?

(2) I actually don't have any preference on this.

(3) I am for the longer names, or else the abbr. p would sometimes mean pull and sometimes push...?

cortner commented 3 months ago

I think p would be ambiguous, but pb and pfwd are fairly clear. That said, there is a good argument for full names too. Hence I am asking for preferences :).

cortner commented 3 months ago

@DexuanZhou and @hjchen1984 - if you have a view I'd be interested in hearing it!!

wcwitt commented 3 months ago

At risk of speaking out of turn, I prefer the more verbose names because I can mostly understand them without having immersed myself in the repo, whereas things like 'pb_' are less immediately interpretable

cortner commented 3 months ago

@wcwitt -- not at all, thank you for chiming in. I somewhat agree with you. My only concern is the rever-over-reverse, i.e. second pullback.

pullback_pullback_evaluate!(...)

looks nasty. But maybe this will be used so rarely that I just shouldn't worry.

DexuanZhou commented 3 months ago

(1) I think keeping Polynomials4ML or splitting off the ACE-related operations into a separate package both works well for me. (3) I prefer shorter names: Pullbacks: pb_evaluate! Double-pullback: pb_pb_evaluate! Push-forward: pfwd_evaluate!.

zhanglw0521 commented 3 months ago

@wcwitt -- not at all, thank you for chiming in. I somewhat agree with you. My only concern is the rever-over-reverse, i.e. second pullback.

pullback_pullback_evaluate!(...)

looks nasty. But maybe this will be used so rarely that I just shouldn't worry.

For that maybe pullback2 would work? When I see pullback2 I will recall ..._ed2 and can then guess the meaning.

cortner commented 3 months ago

Re (1) it seems there is no desire to change the name or to re-organize where code lives. So we will leave it as is for now.

Re (3) What about the following list of functions:

evaluate!
evaluate_ed! 
evaluate_ed2! 
pullback! 
pushforward!

and for 2nd-order, for now experimentally,

pullback2!
pushforward2!

If we need a pullback or pushforward through any function other than evaluate, e.g. a function fun then it needs to be made explicit e.g. via pullback_fun and so forth.

cortner commented 2 months ago

thanks everybody for the comments...