Skip to content

Conversation

@harryswift01
Copy link
Contributor

@harryswift01 harryswift01 commented Jun 13, 2025

Summary

This PR integrates waterEntropy into CodeEntropy, enabling automatic detection and entropy calculation of water molecules.

Changes

Integration of waterEntropy into CodeEntropy:

  • Automatically detects water molecules in the system.
  • Calls _calculate_water_entropy when water is detected.
  • Updates selection_string to exclude water (not water).
  • Logs entropy results via data_logger.

Refactoring and Optimization:

  • Refactored _finalize_molecule_results to run once after all molecules are processed, improving efficiency.
  • Enhanced _calculate_water_entropy for more robust entropy calculations specific to water molecules.
  • Removed _log_residue_data and _log_result from EntropyManager as these are duplications of the add_residue_data and add_results_data functions in the DataLogger class.

Configuration and Testing Enhancements:

  • Added comprehensive test cases to verify correct function call order and sequence.
  • Updated pyproject.toml to ensure all dependencies are current.

Impact:

  • CodeEntropy is now capable of calculating the entropy of molecules within aqueous (water-based) solutions.
  • Improves automation and accuracy in entropy calculations specifically for water molecules.
  • Eliminates the need for manual configuration by removing the water_entropy flag, thanks to automatic detection.
  • Enhances performance and maintainability through streamlined function calls and refactoring.
  • Expands test coverage, improving the reliability and robustness of the entropy calculation pipeline.

- Automatically detecting any water molecules in the universe and calling `_calculate_water_entropy` function and changing the `selection_string` to `not water`
- `waterEntropy` function is called within `_calculate_water_entropy` and results are logged into `data_logger`
- Removal of the `water_entropy` keyword in the `arg_config_manager` as automatic detection is now used
- Additional test cases to check the functions are being called in the correct order and sequence
- Updated `pyproject.toml` to ensure dependencies are up to date
- More robust way of calculating the total entropy of each molecule if it is water
- Refactored how `_finalize_molecule_results` function operates, rather than a call per molecule it is run after all molecules have been processed
- Additional test cases to fully capture the refactored `_calculate_water_entropy` function
@harryswift01 harryswift01 added this to the 1.0.0 release milestone Jun 13, 2025
@harryswift01 harryswift01 self-assigned this Jun 13, 2025
@harryswift01 harryswift01 added the feature request New feature or request label Jun 13, 2025
@harryswift01 harryswift01 linked an issue Jun 13, 2025 that may be closed by this pull request
3 tasks
- Remove duplicated adding and logging results, using `DataLogger` class to exclusivly handle all of the data handling
- Restructured how results are diplayed, users will now see the `resname` rather than an arbitary internal counter to differentiate the molecules
- Changing of `_args.selection_string` to ensure user's input is not overwritten if selection is not `all`
- Updated test cases to reflect the changes made within this commit
Copy link
Member

@jimboid jimboid left a comment

Choose a reason for hiding this comment

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

A great PR, it looks like the water entropy is essentially there and would function. I like the automation of detecting water, though I think this opens up the question which is the opposite of what we had before. Instead of not having water implemented and config options were present to turn it on (even though it wasn't implemented). We have the scenario where users have water present but are only wanting to run structural entropy calcs not being able to turn it off.

@jkalayan what are your thoughts on this? I think having the default as auto as it is with an override option to disable would be useful?

Code looks good, but since we have multiple reviewer requests I won't approve just yet so last one of @jkalayan or @skfegan to review can set the approval please, also give your opinion on the above question before merge.

@skfegan
Copy link
Member

skfegan commented Jun 17, 2025

I tried it with only 3 frames, so the water entropy calculation would not take too long. This gives negative eigenvalues for the protein vibrational entropy calculation and may not be well converged for the water entropy values, but it looks like the data from the water entropy code is being returned.

@jimboid
Copy link
Member

jimboid commented Jun 17, 2025

I tried it with only 3 frames, so the water entropy calculation would not take too long. This gives negative eigenvalues for the protein vibrational entropy calculation and may not be well converged for the water entropy values, but it looks like the data from the water entropy code is being returned.

Can you post the output here so that @jkalayan can take a look, then we can mark this to merge once happy.

@skfegan
Copy link
Member

skfegan commented Jun 17, 2025

program.log

Hope this helps, it is a bit messy as we only have the debug file not the final formatted output, until I run it on a longer bit of the trajectory.

@jkalayan
Copy link
Collaborator

Thanks Sarah, looks like the water entropy outputs are in the log file as expected, I'm happy with this integration of WE to CE

Copy link
Collaborator

@jkalayan jkalayan left a comment

Choose a reason for hiding this comment

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

WE outputs look correct in Sarah's logfile, happy with this integration

@harryswift01 harryswift01 merged commit bcbdb24 into main Jun 18, 2025
7 checks passed
@harryswift01 harryswift01 deleted the 104-integrating-waterentropy branch June 18, 2025 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integrating waterEntropy into CodeEntropy

5 participants