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

test: Add a test for System Coupling related settings #2812

Merged
merged 4 commits into from
May 16, 2024
Merged

Conversation

ochernuk
Copy link
Collaborator

Adding a simple test to check System Coupling related settings. It's probably worth adding a few more, but that can be done separately.

@ochernuk
Copy link
Collaborator Author

ochernuk commented May 13, 2024

@seanpearsonuk @mkundu1 @raph-luc wondering if you could offer any insight here. The test that I added is failing using v24.2.0 Fluent. Looks like it comes down to not being able to download the SCP file that Fluent generates from the container. So, either it's not using the download functionality correctly here:

# download the file locally in case Fluent is in a container

... or the download() function is not working on GitHub.

(the assert on line 96 in the above link fails)

@raph-luc
Copy link
Member

raph-luc commented May 13, 2024

@ochernuk You're right, and it's failing in v241 too not only v242. We definitely need some tests specifically for the File Transfer Service-related methods to give us confidence that it is working properly, I will make sure we bring this up in the team meeting tomorrow if others don't get to this first.

@seanpearsonuk
Copy link
Collaborator

@seanpearsonuk @mkundu1 @raph-luc wondering if you could offer any insight here. The test that I added is failing using v24.2.0 Fluent. Looks like it comes down to not being able to download the SCP file that Fluent generates from the container. So, either it's not using the download functionality correctly here:

# download the file locally in case Fluent is in a container

... or the download() function is not working on GitHub.

(the assert on line 96 in the above link fails)

@hpohekar Is it expected to work? Do we have relevant tests?

@hpohekar
Copy link
Collaborator

hpohekar commented May 14, 2024

@seanpearsonuk @mkundu1 @raph-luc wondering if you could offer any insight here. The test that I added is failing using v24.2.0 Fluent. Looks like it comes down to not being able to download the SCP file that Fluent generates from the container. So, either it's not using the download functionality correctly here:

# download the file locally in case Fluent is in a container

... or the download() function is not working on GitHub.
(the assert on line 96 in the above link fails)

@hpohekar Is it expected to work? Do we have relevant tests?

@seanpearsonuk We are not using any file transfer service by default. In case of container mode, specifically in GitHub the written file will be available in pyfluent.EXAMPLES_PATH directory by default. No need to download it explicitly in the GitHub test.

Right now it is failing at self._solver.download(scp_file_name) call because file_transfer_service is not provided.

If we still want to download it then we need add following code inside the test.

from ansys.fluent.core.utils.file_transfer_service import RemoteFileTransferStrategy
import ansys.fluent.core as pyfluent
from pathlib import Path
pyfluent.USE_FILE_TRANSFER_SERVICE = True
session = pyfluent.launch_fluent(file_transfer_service=RemoteFileTransferStrategy())
file_name = Path(pyfluent.EXAMPLES_PATH) / scp_file_name
session.download(file_name=file_name ) # it will download that file in `pyfluent.USER_DATA_PATH` by default for GitHub and in current working directory if we use wsl or Docker Desktop locally. 

@hpohekar
Copy link
Collaborator

@seanpearsonuk @mkundu1 @raph-luc wondering if you could offer any insight here. The test that I added is failing using v24.2.0 Fluent. Looks like it comes down to not being able to download the SCP file that Fluent generates from the container. So, either it's not using the download functionality correctly here:

# download the file locally in case Fluent is in a container

... or the download() function is not working on GitHub.
(the assert on line 96 in the above link fails)

@hpohekar Is it expected to work? Do we have relevant tests?

@seanpearsonuk

Is it expected to work? - No. It only works if we provide any file transfer service.

Do we have relevant tests? - Yes.

https://github.com/ansys/pyfluent/blob/main/tests/test_file_transfer_service.py#L42

@seanpearsonuk
Copy link
Collaborator

pyfluent.USE_FILE_TRANSFER_SERVICE = True
session = pyfluent.launch_fluent(file_transfer_service=RemoteFileTransferStrategy())

That's great, many thanks for the detailed response, @hpohekar.

@mkundu1 (et al) should file_transfer_service=RemoteFileTransferStrategy() simply override the module-level setting?

@raph-luc raph-luc enabled auto-merge (squash) May 16, 2024 14:54
@raph-luc raph-luc merged commit 2f038e8 into main May 16, 2024
26 checks passed
@raph-luc raph-luc deleted the test/add_syc_test branch May 16, 2024 16:18
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

5 participants