Skip to content

Refactor AdcMethod + ISR(1)-d#208

Open
frieschneider wants to merge 10 commits intomasterfrom
refactor_adcmethod
Open

Refactor AdcMethod + ISR(1)-d#208
frieschneider wants to merge 10 commits intomasterfrom
refactor_adcmethod

Conversation

@frieschneider
Copy link
Copy Markdown
Contributor

Refactor AdcMethod.py:

  • explicitly differentiate between ADC calculation (AdcMethod) and property calculation (IsrMethod)
  • method validation does not rely anymore on creating a list with all valid methods
  • added some minimal tests

Include ISR(1)-d:

densities:

  • reorganized different terms
  • used already available check_doubles_amplitudes function to assess whether doubles excitation are present (this function throws a ValueError -> try except layout)
  • renaming of the functions (e.g. s2s_tdm_adc2 is now called s2s_tdm_isr2)

Modified transition moments and B matrix:

  • ISR(1)-d B matrix: block_orders need to be explicitly set. Update _default_block_orders?
  • renaming (adc -> isr)

no test yet for ISR(1)-d

Comment thread adcc/adc_pp/state2state_transition_dm.py
Comment thread adcc/adc_pp/state2state_transition_dm.py
Comment thread adcc/adc_pp/state_diffdm.py Outdated
Comment thread adcc/adc_pp/state_diffdm_2p.py Outdated
Comment thread adcc/adc_pp/state_diffdm_2p.py Outdated
Comment thread adcc/tests/AdcMethod_test.py Outdated
Comment thread adcc/AdcMatrix.py
Comment thread adcc/AdcMatrix.py Outdated
Comment thread adcc/AdcMethod.py Outdated

def diffdm_adc2_2p(mp, amplitude, intermediates):
dm = diffdm_adc1_2p(mp, amplitude, intermediates) # Get ADC(1) result
def diffdm_isr1_2p(mp, amplitude, intermediates):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this function call isr1s to avoid the repetition for the first-order singles-singles contribution?

Comment thread adcc/AdcMatrix.py
Comment on lines +80 to +82
def _default_block_orders(cls,
method: Method
) -> dict[str, int]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
def _default_block_orders(cls,
method: Method
) -> dict[str, int]:
def _default_block_orders(cls, method: Method) -> dict[str, int]:

Comment thread adcc/AdcMethod.py
Comment on lines +110 to +111
def _base_method(self) -> str:
return self._method_base_name + self.level.to_str()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
def _base_method(self) -> str:
return self._method_base_name + self.level.to_str()
def _base_method(self) -> str:
assert self._method_base_name is not None
return self._method_base_name + self.level.to_str()

Comment thread adcc/AdcMethod.py
Comment on lines +38 to +41
# special levels
TWO_X = "2x"
ONE_S = "1s"
THREE_D = "3d"
Copy link
Copy Markdown
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 good to add a comment for each special level that explains what the level means

Comment thread adcc/AdcMethod.py
Comment on lines +83 to +84
if split and split[0] not in ["cvs"]:
raise ValueError(f"{split[0]} is not a valid method prefix")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we also need to verify that split is at most of length 1 with cvs being the only valid entry, right?

Suggested change
if split and split[0] not in ["cvs"]:
raise ValueError(f"{split[0]} is not a valid method prefix")
valid_prefixes: tuple[str, ...] = ("cvs",)
if len(split) > len(valid_prefixes):
raise ValueError(f"Invalid number of method prefixes provided in {split}.")
if any(pref not in valid_prefixes for pref in split):
raise ValueError(f"Invalid method prefix in {split}.")

Comment thread adcc/AdcMethod.py
Comment on lines +91 to +93
if isinstance(level.value, int):
if level.value <= self.max_level:
return
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if isinstance(level.value, int):
if level.value <= self.max_level:
return
if isinstance(level.value, int) and level.value <= self.max_level:
return

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