Skip to content
This repository has been archived by the owner on Sep 1, 2021. It is now read-only.

extention of calculation of molecular orbitals and electron density for orca written data #31

Merged
merged 1 commit into from
Jan 29, 2016

Conversation

lenkd
Copy link
Contributor

@lenkd lenkd commented Sep 22, 2015

  • depending on the program that creates the data sets now the program uses different normalization routines for calculation of molecular orbitals and the electron density
    (differs between ORCA written files and others like Gaussian)
    -small correction in the d-orbital also for the gaussian-like calculation

@ghutchis
Copy link
Collaborator

@AlbertDeFusco - can you take a look? This is workable, but seems incredibly inelegant by duplicating so much code. Is there no better way to handle the different normalization?

@lenkd
Copy link
Contributor Author

lenkd commented Sep 22, 2015

I would like to have a solution with pointer to functions but I was afraid that some people want to have more straight forward and readable code. Now the calculations for Orca and the others are independent but in the same flow so it is easy to follow which calculations will be done.I would be really thankful if you have any better ideas and I am going to change the code immediately

@AlbertDeFusco
Copy link
Contributor

Thanks, Dagmar!

I compiled and tested and GAMESS and Gaussian look fine but I agree that reducing the duplication would be nice. Is it possible to use the if(m_useOrcaNorm) statement inside initCalculation and the point functions? That way even though the normalization is drastically different the function names shouldn't change.

The more time consuming way is to work out the minimal differences between the equations. I wouldn't advocate that as we could lose readability. For these sort of things I have always greatly appreciated easier understood equations over optimizing the least mathematical operations.

@lenkd
Copy link
Contributor Author

lenkd commented Sep 23, 2015

To have a branch inside the point functions will increase the calculation time noticeable, but I am going to try to shift the different parts into the normalization so we can use the same point function. Another thing is that the calculation for the g,h and i orbitals must not be used for gaussian data etc. so we have to differ anyway for these routines.
Regarding the initCalculation routine this is the most obvious part that is different so there is quiet less double coding.

@cryos
Copy link
Owner

cryos commented Sep 23, 2015

Thanks for working on this, I agree that focusing on the init function seems best, reducing branching in the point functions (even to the point of introducing some branching) would be worth it. I would like to look at other ways to accelerate the point functions, and worked hard to keep any branching not needed out of them.

@ghutchis
Copy link
Collaborator

ghutchis commented Jan 7, 2016

I'm inclined to merge this as-is and revisit for Avogadro v2.

@cryos
Copy link
Owner

cryos commented Jan 29, 2016

Sounds good, I have been working with the NWChem folks quite a bit, and looking at code to do reordering which is different there too. I think we need to figure this out, but if Geoff wants this functionality in v1 we should do as suggested.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants