Conversation
|
If you rebase this against the current |
|
Done |
|
Now the clang-formatter should be happy too |
|
I still don't fully understand the purpose of this PR. What do you mean by "not accessible" from the In your project, you could just import from sub-modules directly, e.g., from adcc.workflow import validate_state_parametersrather than exposing, e.g., Am I missing something? |
|
This is true. The important part in this PR is the cpp part, where I introduced the new function set_lt_scalar, which is something I cannot hack together with a few lines of code outside of adcc. Since I needed the modules for the external package, I thought they should be exposed through |
| } | ||
|
|
||
| static ten_ptr set_lt_scalar(const py::float_ n) { | ||
| auto adcmem_ptr = std::shared_ptr<AdcMemory>(new AdcMemory()); |
There was a problem hiding this comment.
I'm a bit worried about the following:
- Creating a new, possibly dangling
AdcMemoryinstance here. AFAIK, this is usually only created once and used throughout. - Importing
libtensordirectly in the pybind export file. Usually, we only importlibtensorinTensorImpl.cc.
If you have a second, @mfherbst, what do you think about it?
If I understand correctly, this functionality just enables using a single number as item in an amplitude vector. With some boilerplate, this could possibly very easily achieved on the Python level, right?
…r function is now used. Furthermore, instead of just a scalar an arbitrary 1d array can be initialized. Using and mathematical operations like arbitrary libtensors can now be initialized and set from the python side. This doesn't support symmetry though.
|
I've addressed your points in the new commit and also extended the initialization from a single scalar to a 1d array. I've searched through everything, which is imported to the python side from libadcc, but could't find anything to initialize libtensor arrays of arbitrary dimension and length from the python side. If there is anything, I've missed it. |
2 participants
I rewrote pull request #153 as a separate repository called polaritonic_adcc on my home github, where I noticed that some functionalities I needed were not accessible from the adcc module. Before I simply copy, paste and reference a lot of the adcc code, it would be much easier if this pull request got accepted, in case somebody else also wants to build a similar project.
I also had to add an additional function to the libadcc site, which lets you set a libtensor array with one entry from the python side. I didn't really see an other option, but if you guys have a good idea for a workaround, I could remove it from this pull request.