Skip to content

provide lambda for writing mesh files - #141

Merged
tcaduser merged 1 commit into
r2.8.0from
refactorWrite6
Jun 7, 2024
Merged

tcaduser merged 1 commit into
r2.8.0from
refactorWrite6

Conversation

@tcaduser

@tcaduser tcaduser commented Jun 7, 2024

Copy link
Copy Markdown
Member

Description of changes:

Instead of a regex, use an lambda to determine whether a field should be written to a Tecplot or VTK file.

devsim.write_devices(
    file="mesh2d_reduced.tec",
    type="tecplot",
    include_test=lambda x: x in ("NetDoping",),
)

Also remove floops/floods writer.

Licensing

To maintain the integrity of the project, we require contributions to be licensed under the Apache License Version 2.0. Please acknowledge this by checking the box below. By checking the box, you also acknowledge that you have the authority to license your contribution in this manner. You should also ensure that any new file you add contains "# SPDX-License-Identifier: Apache-2.0" in the header.

    remove floops writer
    update version string
    copy over docstring
    python formatting
@tcaduser
tcaduser requested a review from simbilod June 7, 2024 22:59
Comment thread CHANGES.md

Based on a contribution by [@simbilod](https://github.com/simbilod) `devsim.write_devices` now supports reducing the file size of data files by specifying a callback function to reduce data usage. This is through the `include_test` option. This option is a function that takes a string and returns `True` if the model should be written. It is only supported for the `tecplot` and `vtk` formats.

An example is provided in `testing/XXXXX.py`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should XXXXX be mesh2d.py ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

thanks, I'll clean this up at the end.

Comment thread src/MathEval/MathEval.cc

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It is not immediately obvious to me how this change relates to the commit message and PR. Having a separate commit with a message explaining why this is removed could be good for documentation purposes. But we can just merge this time :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The format was very used. It was supposed to be compatible this FLOOPS/FLOODS simulator output for visualization in a software named Postmini

@simbilod

simbilod commented Jun 7, 2024

Copy link
Copy Markdown
Collaborator

Yes it is better to simplify to a single flag and a single share object reused across the writers to handle this. Thanks!

@tcaduser

tcaduser commented Jun 7, 2024

Copy link
Copy Markdown
Member Author

The stuff with MathEval were dead code that was never used.

@tcaduser
tcaduser merged commit 73b61de into r2.8.0 Jun 7, 2024
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