Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ocean tutorial: set ngkpt the same as nkpt #170

Merged
merged 3 commits into from
Apr 14, 2023
Merged

Conversation

deyulu
Copy link
Collaborator

@deyulu deyulu commented Apr 13, 2023

No description provided.

@deyulu
Copy link
Collaborator Author

deyulu commented Apr 13, 2023

@FCMeng I added one line in ocean.py and test failed. Can you take a look if I broke anything?

@FCMeng
Copy link
Collaborator

FCMeng commented Apr 13, 2023

It seems the change you made is not the reason for the failure. @matthewcarbone The error message is like "ERROR: Could not find a version that satisfies the requirement codecov (from versions: none)". Do we need to modify the pipeline a bit?

@matthewcarbone
Copy link
Contributor

@FCMeng Yeah not exactly sure what's going on with that. If you have any ideas let me know, but it's not your code or the branch that's causing the issue.

@deyulu
Copy link
Collaborator Author

deyulu commented Apr 13, 2023

@matthewcarbone @FCMeng We need to solve this issue before the workshop, as the current nkpt setting is incorrect.

@matthewcarbone
Copy link
Contributor

Truely nonsensical. Codecov was removed from PyPi (see e.g. here for some more discussion). It's now fixed on master. Just rebase this branch and it'll fix the tests.

@deyulu deyulu changed the title Ocean tutorial: set ngkpt the same as nkpt deprecated: Ocean tutorial: set ngkpt the same as nkpt Apr 14, 2023
@deyulu deyulu changed the title deprecated: Ocean tutorial: set ngkpt the same as nkpt Ocean tutorial: set ngkpt the same as nkpt Apr 14, 2023
@deyulu
Copy link
Collaborator Author

deyulu commented Apr 14, 2023

@matthewcarbone Thank you. I rebased the branch and most of the tests passed with one still running. It looks like I also need to add the ocean tutorial notebook.

@deyulu
Copy link
Collaborator Author

deyulu commented Apr 14, 2023

Does anybody know why [Run unit testing suite (macOS-latest, 3.11)] takes so long? It has run 1.5 hours and still not finished.

@FCMeng
Copy link
Collaborator

FCMeng commented Apr 14, 2023

It is done on my side.

@codecov-commenter
Copy link

codecov-commenter commented Apr 14, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 🎉

Comparison is base (b40aa03) 80.75% compared to head (c0a2518) 80.77%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #170      +/-   ##
==========================================
+ Coverage   80.75%   80.77%   +0.02%     
==========================================
  Files          12       12              
  Lines         930      931       +1     
==========================================
+ Hits          751      752       +1     
  Misses        179      179              
Impacted Files Coverage Δ
lightshow/parameters/ocean.py 83.80% <100.00%> (+0.11%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@matthewcarbone
Copy link
Contributor

matthewcarbone commented Apr 14, 2023

Does anybody know why [Run unit testing suite (macOS-latest, 3.11)] takes so long? It has run 1.5 hours and still not finished.

@deyulu it's showing successful in 3 minutes. Maybe a glitch on your end?

@matthewcarbone
Copy link
Contributor

@deyulu can you add the changes from your Ocean notebook to the only one in master right now? I combined them before.

@deyulu
Copy link
Collaborator Author

deyulu commented Apr 14, 2023

@matthewcarbone Changes in k-mesh doesn't affect the notebook. No change to be added. I didn't notice that you already combined them. However, I need to add "import lightshow" for the notebook to work.

@matthewcarbone
Copy link
Contributor

@deyulu No problem, and I see you removed the notebook I was referring to. Now there's only a single change as expected. Rebasing!

@matthewcarbone matthewcarbone merged commit aa391c1 into master Apr 14, 2023
@matthewcarbone matthewcarbone deleted the ocean_tutorial branch April 14, 2023 11:23
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.

None yet

4 participants