Skip to content

Conversation

Copy link

Copilot AI commented Jan 4, 2026

Plan for Fixing Birch-Murnaghan EOS

  • Understand the difference between Murnaghan and Birch-Murnaghan equations
  • Review current implementation of BMEOS function
  • Implement the correct 3rd-order Birch-Murnaghan equation
  • Add Murnaghan equation as a separate function (for compatibility)
  • Update documentation to reflect the correct equation
  • Test the implementation and validate correctness
  • Update scan_pressures function documentation
  • Run code review and fix issues
  • Run security checks (no issues found)
  • Final verification complete
  • Make internal function naming consistent (invbmeos → invmurneos)

Summary

Successfully implemented the correct 3rd-order Birch-Murnaghan equation of state, replacing the incorrect Murnaghan equation that was previously used. All tests pass, documentation is updated, and internal naming is now consistent.

Key Changes:

  1. BMEOS function: Now implements true 3rd-order Birch-Murnaghan EOS
  2. MurnaghanEOS function: Added as separate function for reference/compatibility
  3. Documentation: Updated in elastic.py, init.py, and lib-usage.rst
  4. Internal naming: Renamed invbmeos to invmurneos for consistency
  5. Validation: All unit tests pass, no security issues found
Original prompt

This section details on the original issue you should resolve

<issue_title>Birch-Murnaghan equation of state</issue_title>
<issue_description>Thank you so much for sharing your code. I met some issues in the code. I think the equation used in file 'elastic.py' function 'BMEOS' is not Birch-Murnaghan equation.
It is Murnaghan equation: https://en.wikipedia.org/wiki/Murnaghan_equation_of_state
The correct equation is: https://en.wikipedia.org/wiki/Birch%E2%80%93Murnaghan_equation_of_state </issue_description>

Comments on the Issue (you are @copilot in this section)

@jochym You are right. I will make the correction in the text. Later, I will implement B-M version as well. (I will also accept PR if you have time ;). Tanks, @Ruoyan0103 !

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 3 commits January 4, 2026 20:20
Co-authored-by: jochym <5993422+jochym@users.noreply.github.com>
Co-authored-by: jochym <5993422+jochym@users.noreply.github.com>
Co-authored-by: jochym <5993422+jochym@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix Birch-Murnaghan equation in elastic.py Fix BMEOS to use Birch-Murnaghan equation instead of Murnaghan equation Jan 4, 2026
Copilot AI requested a review from jochym January 4, 2026 20:26
Copy link
Owner

@jochym jochym left a comment

Choose a reason for hiding this comment

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

Make internal function naming consistent

# Inverse Murnaghan EOS to get volumes from pressures
# This will work only in limited pressure range p>-B/B'.
# Warning! Relative, the V0 prefactor is removed.
def invbmeos(b, bp, x):
Copy link
Owner

Choose a reason for hiding this comment

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

This should also beem modified (mame). This is inverse Murnaghan not Birch- Murnaghan @copilot

Copy link
Author

Choose a reason for hiding this comment

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

Renamed invbmeos to invmurneos and updated the comment on line 624 to say "Murnaghan EOS" instead of "B-M EOS" for consistency. Fixed in fd4faaa.

Co-authored-by: jochym <5993422+jochym@users.noreply.github.com>
Copilot AI requested a review from jochym January 5, 2026 04:26
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.

Birch-Murnaghan equation of state

2 participants