Skip to content

Conversation

YuiKasagi
Copy link
Collaborator

@YuiKasagi YuiKasagi commented May 22, 2025

Since I would like to use the FeH line list by Hargreaves et al. (2010), I implemented a custom API by referencing radis.api.dbmanager and exojax.spec.api.

  • The line list contains many unassigned lines, where only the line center wavenumber and intensity are provided. For missing values such as the Einstein A coefficient and upper state J, I assigned either constants or computed estimates.
  • Additionally, the line list does not include a partition function. To address this, I used the partition function from ExoMol (via MoLLIST). Specifically, I first load the MdbExoMol for FeH, and then overwrite certain attributes using MdbHargreaves.

Please see the sample code documents/userguide/customapi.ipynb for its usage.
https://github.com/YuiKasagi/exojax/blob/api_mdb_external/documents/userguide/customapi.ipynb

@YuiKasagi YuiKasagi requested a review from HajimeKawahara May 22, 2025 07:02
@YuiKasagi YuiKasagi added the enhancement New feature or request label May 22, 2025
@HajimeKawahara HajimeKawahara added this to the Version 2.1 milestone May 24, 2025
@HajimeKawahara HajimeKawahara requested a review from Copilot May 24, 2025 07:24
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a custom molecular database API for the FeH line list by Hargreaves et al. (2010), including utilities to fetch, parse, and convert it into ExoMol‐compatible format.

  • Introduces MdbHargreaves in customapi.py (with fetching, parsing, intensity/A‐coefficient estimation, and activation logic)
  • Adds HargreavesDatabaseManager under dbmanager.py and a test harness (mock_mdbHargreaves) in emulate_mdb.py
  • Provides basic unit tests for set_wavenum and line‐strength sums in customapi_test.py

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/unittests/spec/api/customapi_test.py New tests for set_wavenum and Hargreaves line strengths
src/exojax/test/emulate_mdb.py Added mock_mdbHargreaves and related import
src/exojax/spec/qstate.py Added branch_to_number helper
src/exojax/spec/dbmanager.py New CustomDatabaseManager and HargreavesDatabaseManager
src/exojax/spec/customapi.py Implementation of MdbHargreaves and related utilities
Comments suppressed due to low confidence (4)

src/exojax/spec/customapi.py:102

  • The database name string is not formatted; it literally contains {molecule}. Use an f-string (e.g. f"Hargreaves2010-{molecule}") or format before passing.
super().__init__( name="Hargreaves2010-{molecule}",

tests/unittests/spec/api/customapi_test.py:24

  • There are no unit tests covering convert_to_exomol logic, NaN handling for jlower or A‐coefficient estimation paths. Consider adding tests for those branches.
def test_line_strength_hargreaves():

src/exojax/spec/qstate.py:36

  • [nitpick] This error message is generic; consider including the invalid values or their indices to aid debugging, e.g. f"Invalid branch values at positions {invalid_indices}: {branch_str[invalid_indices]}".
raise ValueError("Invalid or NaN branch values found.")

src/exojax/test/emulate_mdb.py:120

  • The code uses os.getcwd() and os.path.exists but os is not imported. Add import os at the top of the file.
target_dir = os.getcwd() + "/" + molecule

@YuiKasagi
Copy link
Collaborator Author

I made the following modifications in response to @HajimeKawahara's comments:

  • Created data/opacity/FeH_Hargreaves2010.csv (265KB)
  • Added the reference in both data/README and the header of the csv file
  • Modified the code to load the CSV file by default instead of downloading the line list from the web
  • Added comments at the end of customapi.ipynb explaining that the default line strength scaling factor is set to 1/3, as referenced in papers on the Sonora model

@YuiKasagi
Copy link
Collaborator Author

  • Updated MdbHargreaves.activate_with_exomol so that it now extends the line list by default, returning a combination of the ExoMol and Hargreaves line lists.

Copy link
Owner

@HajimeKawahara HajimeKawahara left a comment

Choose a reason for hiding this comment

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

Thank you!

@HajimeKawahara HajimeKawahara merged commit 7ea0872 into HajimeKawahara:develop May 29, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants