dftlibs / xcfun

XCFun: A library of exchange-correlation functionals with arbitrary-order derivatives
https://dftlibs.org/xcfun/
Mozilla Public License 2.0
57 stars 32 forks source link

A parser for "mode" and "var" #46

Closed ilfreddy closed 4 years ago

ilfreddy commented 6 years ago

Issue

Setting up enum xc_vars and enum xc_mode is not very user-friendly for the time being. Especially vars. I think there should be a user-friendly version of xc_eval_setup which takes some sensible parameters

It seems to me that with these input parameters specified (there might be some redundancy in my suggestions) one can assign both vars and mode unambiguously.

Possible solution

It should be enough with a user friendly variant of xc_eval_setup

Something like enum xc_vars xc_var_setup(const string &funcType, const string const string &densType.... { if ... else if .... else if ... ... else.. return XC_CORRECT_VAR }

bast commented 6 years ago

I just remembered one possible motivation for why it is done this way as it is now: it makes it easier to do this across languages (e.g. from Fortran). Easier to send integer parameters than strings but with iso_c_binding it should be possible to do this, too.

So what you suggest is an interface that returns you the right integer parameter? Or are you rather after an interface where you don't see these parameters at all and they stay on the library side?

ilfreddy commented 6 years ago

What I want is an interface where it is easy to go from let's say "I want a metaGGA functional with spin polarization and the laplacian" without having to hard-code in the caller the corresponding enum XC_A_B_GAA_GAB_GBB_LAPA_LAPB_TAUA_TAUB We can very well do it with integers if well documented: each of the parameters I suggested can be coded with 3/4 integers each (eg: LDA=0 GGA=1, MetaGGA=2). Then we wrap everything into a routine which returns the correct enum.

ilfreddy commented 6 years ago

Random remarks:

bast commented 6 years ago

I think I see your point. How would you treat NX vs GNN? Both are GGA.

ilfreddy commented 6 years ago

I guess we need yet another option. GAMMA vs EXPLICIT?

ilfreddy commented 6 years ago

BTW, if we assign explicit values to the enum xc_vars, it becomes possible to enquire the functional about specific settings by simple bit manipulation with functions like: bool is_func_lda(xc_functional fun)

uekstrom commented 6 years ago

You can query a functional by calling xc_eval_setup with a vars argument and check the return value. For example metaGGA is not a well defined term in general (although it can of course be made well defined in the xcfun case).

uekstrom commented 6 years ago

I think it's good that you are improving the interface but I would like to comment on this passage:

What I want is an interface where it is easy to go from let's say "I want a metaGGA functional with spin polarization and the laplacian" without having to hard-code in the caller the corresponding enum XC_A_B_GAA_GAB_GBB_LAPA_LAPB_TAUA_TAUB''

To use this functional you still have to compute all the input quantities and place them in the correct array location. Hence the application program will be hard-coded for this method of calling xcfun anyway. I was thinking about some way to query the array layout but for performance reasons I suspect you always want this hard coded.

ilfreddy commented 6 years ago

I thought xc_eval_setup would set the values of vars and mode. How would you use it to simply check whether let's say a functional is GGA or whether it uses gamma rather than explicit partial derivatives?

uekstrom commented 6 years ago

Ah, you mean that the setup is done in one part of the code and then later you want to check what you actually set up? That is not possible today but would be nice to have.

ilfreddy commented 6 years ago

Indeed!

bast commented 6 years ago

I agree. With this I would not have to carry around the information through my "outside" code that some functional is GGA and I need gradients.

uekstrom commented 6 years ago

You can expose the vars (i.e. something from the xc_vars enum) from the functional object. I can almost bet that you will do a switch (or whatever Fortran calls this language construct) on this quantity later on. Having a way to ask "do I need gradients" is ambiguous because you will not know what gradient quantities you need, and you will then also ask do I need just the square or the individual components etc.

ilfreddy commented 6 years ago

Do you think it would simply be better to expose the vars? How does one do it from the functional object (possibly a silly question...)? At any rate, if one chooses to do so, I think it would be best to have values which encode the information in bits such it can be easily extracted without a monstruous switch every time. Concerning the questions such as "do i need gradients?", I think it is a matter of formulating them unambiguously. Possible examples: is_gga, need_expicit_gradient_components, need_two_densities, need_total_density

uekstrom commented 6 years ago

You could add a function to the API

enum xc_vars xc_get_vars(xc_functional fun);

that simply returns the vars value used to set up the functional. I agree it could have some more logical bit patterns than the linear enumeration we have now. Maybe a separate header with #defines instead of the enum, so that it can be included from Fortran as well.

bast commented 6 years ago

It is no problem to expose enum to Fortran but the cost might be that we need to define it in two places. Would be nice to define the interface in one place. But we can also generate the Fortran interface from the C interface.

bast commented 6 years ago

I think what I tried to say is that I believe we should not let Fortran limit us in creating a nice interface (not implying that the current interface is not nice). I think we should adapt Fortran to whatever we choose and not so much vice versa.

uekstrom commented 6 years ago

You can see the "logic" of the variable sets in densvars.hpp in the densvars constructor. The use of switch fallthrough shows which sets are supersets of other sets. By the way I see that there is a (no-impact but still) bug in the use of tau for case XC_N_2ND_TAYLOR, so maybe a more logical scheme would be beneficial...

robertodr commented 5 years ago

@ilfreddy was this solved?

robertodr commented 4 years ago

@ilfreddy I think you solved this with the addition of the xc_user_eval_setup function to the API. In #98 I have further added the API functions xcfun_which_mode and xcfun_which_vars that return the correct enum value for the mode and the variables. Please close the issue if no longer relevant.