Skip to content

Include mass weigthing for VP for average mapping scheme#1

Open
DRosen285 wants to merge 4 commits intomainfrom
mass_reweighting
Open

Include mass weigthing for VP for average mapping scheme#1
DRosen285 wants to merge 4 commits intomainfrom
mass_reweighting

Conversation

@DRosen285
Copy link

Title says it all

@DRosen285 DRosen285 requested a review from clarktemple03 July 25, 2022 15:54
Copy link
Contributor

@clarktemple03 clarktemple03 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a few things such as inclusion of masses into the routine. Also changed the variable names to resemble more physical quantities.
The biggest thing is if the masses need to be weighted by their mapping_matrix component. I added a new commit to do this but see if it makes sense

n_beads: int = 2,
volume: float = 10,
mapping: str = "slice",
masses: np.ndarray = None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be appropriate to have a mass index in case beads are not all the same

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I just find it confusing, that they keyword "None" actually corresponds to an array of ones.

@DRosen285
Copy link
Author

I was a little distracted the last few days. Sorry about that. I agree with most of the changes you made. I also made a little adjustment to the mass calculation of a CG bead. I used the one defined by Noid in Eq. 29 in the MS-CG I paper. They write that this equation has to be fulfilled by a CG model which is consistent with a refrence model. Having said that, it changes the absolute value of the vibrational power, but the same optimal mapping scheme gets identified as with your previous calculation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants