diff comparison from last pull request by HopeBestWorld · Pull Request #8 · symbiotic-engineering/OpenFLASH

HopeBestWorld · 2025-06-25T22:06:18Z

No description provided.

review-notebook-app · 2025-06-25T22:06:23Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

rebeccamccabe · 2025-06-25T23:22:31Z

-## Installation
+1.  **Install the package:**
+    ```bash
+    pip install git+[https://github.com/symbiotic-engineering/semi-analytical-hydro.git](https://github.com/symbiotic-engineering/semi-analytical-hydro.git)


Shouldn't this be open flash now that it's on pypi?

rebeccamccabe · 2025-06-25T23:23:43Z

+## References

-### Install from the `CS_group` Branch
+The following publications are relevant to this package:


Add citations for the JOSS paper and the JFM paper (without writing JOSS or JFM since they haven't accepted it yet), you can add our names and the title and say "in prep", see the MDOcean readme as example

rebeccamccabe · 2025-06-25T23:25:54Z

+.. autodata:: m0
+   :annotation: = 1
+
+   Radial wave number.


Just called "wavenumber", more often denoted with k

to be consistent with your description of omega, it's "wavenumber of the incident wave in 1/m"

rebeccamccabe · 2025-06-25T23:27:25Z

+   Radial wave number.
+
+.. autodata:: n
+   :annotation: = 3


rebeccamccabe · 2025-06-25T23:27:37Z

+   Related to a mode or term index.
+
+.. autodata:: z
+   :annotation: = 6


rebeccamccabe · 2025-06-26T01:39:20Z

-        # Convert the complex number to a dictionary
+                    hydro_p_terms[i,j] = 0
+
+        # Now return per-mode values


we have previously used mode to refer to the harmonics of the m_k's, but here mode means the mode of motion / degree of freedom. Clarify the difference in comments, or perhaps rename mode to dof here.

rebeccamccabe · 2025-06-26T01:43:20Z

+        imag_coeffs = np.zeros(num_modes)
+
+        for mode_index in range(num_modes):
+            if mode_index < hydro_h_terms.shape[0] and mode_index < hydro_p_terms.shape[0]:


why is this conditional necessary? add comment, I don't remember why this would be the case. Would hydro_h_terms.shape[0] and hydro_p_terms.shape[0] ever be different?

rebeccamccabe · 2025-06-26T01:47:11Z

+        plt.show() # final display command

-    def run_and_store_results(self, problem_index: int, m0) -> Results:
+    def run_and_store_results(self, problem_index: int, m0_values: np.ndarray) -> 'Results': 


use from __future__ import annotations so you can type with Results (no string) even before it's defined

rebeccamccabe · 2025-06-26T01:47:39Z

+    def run_and_store_results(self, problem_index: int, m0_values: np.ndarray) -> 'Results': 
        """
-        Perform the full MEEM computation and store results in the Results class.
+        Perform the full MEEM computation for a *list of frequencies* and store results


array, not list

rebeccamccabe · 2025-06-26T01:59:45Z

+            freq_idx_in_problem = freq_to_idx.get(m0)
+            if freq_idx_in_problem is None:
+                # This should ideally not happen if m0_values are a subset of problem.frequencies
+                print(f"  Warning: m0={m0:.4f} not found in problem.frequencies. Skipping calculation.")


seems like the code structure should be tweaked so this is not a possibility: either problem.frequencies or m0_values is used but not both. My preference would be problem.frequencies

If we need to extract m0_values for some reason ie plottng, extract it from problem.frequencies so it's consistent

rebeccamccabe · 2025-06-26T02:34:19Z

+
    constant = - heaving[i] * a[i]/(2 * (h - d[i]))
    if k == 0:
        if m0 * h < 14:


use M0_H_THRESH=14 or similar constant definition so it can be changed

rebeccamccabe · 2025-06-26T02:38:06Z

+        # ensure r_array doesn't contain 0 if R_2n is called for r=0
+        return 0.5 * np.log(r_array / a[i])
+    else:
+        return besselk(0, lambda_ni(n, i, h, d) * r_array) / besselk(0, lambda_ni(n, i, h, d) * local_scale[i])


don't you need a value error if n<0 here like you have above?

rebeccamccabe · 2025-06-26T02:41:45Z

+def Z_k_e_vectorized(k, z_array, m0, h, NMK, m_k_arr): # Changed 'z' to 'z_array'
+    local_m_k = m_k(NMK, m0, h) 
+    if k == 0:
+        if m0 * h < 14:


use same const as suggested above

rebeccamccabe · 2025-06-26T13:52:30Z

This has wrong eqns in it, but not a worry because it will be deleted and the multi equations is correct

rebeccamccabe · 2025-06-26T13:53:08Z

delete this too since two-cylinder only

rebeccamccabe · 2025-06-26T13:57:15Z

+Below are the constants defined in this module:

-mass = 5.0  # Example mass in kg
+.. autodata:: g


use the constant g and rho so the user doesn't have to specify them, and can override them if they wish. Other constants like m0, geometry, etc should not go in constants, it's when they create the problem, so delete them from here.

rebeccamccabe · 2025-06-26T13:58:13Z

+.. autofunction:: lambda_n2
+   :noindex:
+
+   Calculates the eigenvalue :math:`\lambda_n` for the middle fluid domain (Region 2).


Not relevant since deleting.

rebeccamccabe · 2025-06-26T14:04:22Z

+                if bd == 0:  # One cylinder case (Inner-Exterior directly)
+                    for n in range(N_left): # Inner harmonics (N)
+                        A[row_offset + n][col_offset + n] = (h - d[bd]) * R_1n(n, a[bd], bd, h, d, a)
+                        for m in range(M_right): # Exterior harmonics (K)


This is fixed in multi_MEEM notebook. Transfer this fix over to equations instead of engine.

rebeccamccabe · 2025-06-26T14:41:49Z

@@ -57,7 +57,7 @@ def make_domain_list(self) -> Dict[int, 'Domain']:
                'h': h,


don't allow h, di, and a to be independently passed domain params, but the geometry should auto create the domain objects with these properties so it's consistent.

We want Bimali's functions to discretize the slant in different ways (left, right center, etc) as methods of geometry that it uses when turning r,z coordinates (of the body, representing a curve or other non MEEM supported thing) into a list of domains

we want geometry (not problem as in the UML diagram) to have a property that is a logical array, of size num_domains x num_domains, saying whether a domain is next to each other domain.

rebeccamccabe · 2025-06-26T15:13:01Z

+    all_potentials_batch_data = [] # <--- NEW: List to store potential data for all frequencies/modes
+
+    print("\n--- Starting Batch Processing for Multiple Frequencies ---")
+    for i, current_m0_val in enumerate(meem_problem.frequencies):


have the for loop here not be necessary for the user to do, you can just call engine.solve and it solves all the problems in the list

rebeccamccabe · 2025-06-26T15:13:17Z

+
+    # --- 3. Create a MEEMProblem Instance and set multiple frequencies ---
+    meem_problem = MEEMProblem(geometry=geometry)
+    meem_problem.set_frequencies_modes(analysis_frequencies, analysis_modes)


analysis modes is redudant

rebeccamccabe · 2025-06-26T15:17:36Z

-                #AttributeError: 'Domain' object has no attribute 'r_coordinates'
-                #'r': domain.r_coordinates,
-                #'z': domain.z_coordinates
                'r': geometry_instance.r_coordinates,


Users should be able to pass a desired r/z vector for visualization (or maybe just a resolution and we create it based on the geometry)

rebeccamccabe · 2025-06-26T15:19:49Z

+    damping_matrix = np.array(collected_damping).reshape(num_frequencies, num_modes)
+
+    # --- 7. Instantiate Results and Store Data ---
+    results_obj = Results(geometry, frequencies_for_results, modes_for_results)


use the method that creates results for you, instead of creating it yourself

rebeccamccabe · 2025-06-26T15:39:18Z

+    problem_cache = engine.cache_list[problem] # Access the existing cache
+
+    if problem_cache is None:
+        print("ERROR: problem_cache is None!")


should this be an actual error?

rebeccamccabe · 2025-06-26T15:40:56Z

+                'data': formatted_potentials_for_batch
+            })
+
+        except np.linalg.LinAlgError as e:


error handling should happen in the meem engine, not the test

rebeccamccabe · 2025-06-26T15:47:09Z

Shouldn't this test, in addition to doing mocks, also test that the actual results for the non-first (cached) problem are the same as that problem evaluated without any caching (using the original _full method)

rebeccamccabe · 2025-06-26T15:49:58Z

+    "Intended Audience :: Science/Research",
+    "Topic :: Scientific/Engineering :: Physics",
+]
+dependencies = [


instead of super specific versions and all packages from an env export, this should be a minimal list of dependencies with inequalities

rebeccamccabe · 2025-06-26T15:51:08Z

see comment on pyproject.toml

rebeccamccabe · 2025-06-26T19:32:37Z

great high level walkthrough, can we also include a plot of the hydro coeffs vs omega here?

…nb, and cache files

Fix 100mb pypi build

JFM paper submitted

Anaconda build

JFM thesis version: conditionals and reference specifiers

Update LICENSE

rebeccamccabe reviewed Jun 26, 2025

View reviewed changes

HopeBestWorld temporarily deployed to github-pages September 18, 2025 20:47 — with GitHub Pages Inactive

HopeBestWorld temporarily deployed to github-pages October 16, 2025 23:21 — with GitHub Pages Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 02:44 — with GitHub Pages Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 03:00 — with GitHub Pages Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 03:50 — with GitHub Pages Inactive

HopeBestWorld had a problem deploying to github-pages October 24, 2025 04:16 — with GitHub Actions Failure

HopeBestWorld temporarily deployed to github-pages October 24, 2025 04:16 — with GitHub Pages Inactive

HopeBestWorld had a problem deploying to github-pages October 24, 2025 04:22 — with GitHub Actions Failure

HopeBestWorld temporarily deployed to github-pages October 24, 2025 04:23 — with GitHub Pages Inactive

HopeBestWorld had a problem deploying to github-pages October 24, 2025 04:33 — with GitHub Actions Failure

HopeBestWorld temporarily deployed to github-pages October 24, 2025 04:33 — with GitHub Pages Inactive

HopeBestWorld had a problem deploying to github-pages October 24, 2025 04:39 — with GitHub Actions Failure

HopeBestWorld temporarily deployed to github-pages October 24, 2025 04:40 — with GitHub Pages Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 04:48 — with GitHub Actions Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 04:48 — with GitHub Pages Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 05:00 — with GitHub Actions Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 05:00 — with GitHub Pages Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 05:09 — with GitHub Actions Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 05:09 — with GitHub Pages Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 15:22 — with GitHub Actions Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 15:23 — with GitHub Pages Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 15:38 — with GitHub Actions Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 15:39 — with GitHub Pages Inactive

HopeBestWorld temporarily deployed to github-pages October 24, 2025 16:19 — with GitHub Actions Inactive

rebeccamccabe and others added 30 commits May 29, 2026 17:38

Sync pubs/JFM with Overleaf project

aec1061

fix \[ \] brackets in appendix equations

d483327

run jfm notebooks for dissertation

7b45855

add jfm to label names to avoid collisions

ac95716

adding jfm: to labels in forgotten places

0ab0d2c

replace tabs with spaces in .bib so bibcop works

2930efa

only show bibliography in non-dissertation version

ad4b3c0

manually sync bibs with overleaf

09c2ba6

Release

b8293aa

Create new project release submitted-paper

5a96efc

Restore readme

48f4658

Fix DOI badge url

600ef1a

create dev calkit environment

5da984e

reduce pypi upload size from >100MB to 2MB by excluding dev, png, ipy…

9b46c14

…nb, and cache files

Merge pull request #89 from symbiotic-engineering/fix-100mb-pypi-build

1f1a4ea

Fix 100mb pypi build

Merge remote-tracking branch 'origin/main' into jfm-paper

4898b24

Readme make separate code/project zenodo badges

dad8766

Merge pull request #88 from symbiotic-engineering/jfm-paper

00676fe

JFM paper submitted

Merge remote-tracking branch 'origin/main' into jfm-thesis

773bd42

Run pipeline

2135393

make sure anaconda build/upload step happens in right env

22941a4

Add activate-environment step to publish workflow

0a59675

Merge pull request #91 from symbiotic-engineering/anaconda-build

44604c6

Anaconda build

rerun with python 3.12

5a7301b

fix location of dissertation \fi to be after begin document

a235a0c

clarify figure caption about ground truth

8c7cc7b

toggle between british and us spellings withmacro

9139757

Merge pull request #90 from symbiotic-engineering/jfm-thesis

f388537

JFM thesis version: conditionals and reference specifiers

Update LICENSE

ad9c77f

Merge pull request #92 from symbiotic-engineering/collin-treacy-patch-1

706425f

Update LICENSE

		@@ -57,7 +57,7 @@ def make_domain_list(self) -> Dict[int, 'Domain']:
		'h': h,

Conversation

HopeBestWorld commented Jun 25, 2025

Uh oh!

review-notebook-app Bot commented Jun 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants